Bug 186272

I don't see any errors in the web inspector. If I inspect the individual resources, it just says "resource has no content." What error do you see...?

(In reply to Michael Catanzaro from comment #1) > I don't see any errors in the web inspector. If I inspect the individual > resources, it just says "resource has no content." What error do you see...? Go to "Network" tab and click on any of the failed jpgs. It says "An error ocurred trying to load the resource". Anyway that's not the point, the images are shown perfectly in FF or in Chrome in the same laptop.

Could it be that dell is serving jpx for us because of the user agent?

(In reply to Carlos Garcia Campos from comment #3) > Could it be that dell is serving jpx for us because of the user agent? Hm, well Epiphany can download the images if I download them directly, but eog cannot display them because they are not JPEG images, even though they use the .jpg extension. Maybe? Probably? kmart.com is broken with the same problem, bug #180995. I closed that WONTFIX because I could not find a user agent quirk that would resolve the issue.

(In reply to Michael Catanzaro from comment #4) > (In reply to Carlos Garcia Campos from comment #3) > > Could it be that dell is serving jpx for us because of the user agent? > > Hm, well Epiphany can download the images if I download them directly, but > eog cannot display them because they are not JPEG images, even though they > use the .jpg extension. > > Maybe? Probably? > > kmart.com is broken with the same problem, bug #180995. I closed that > WONTFIX because I could not find a user agent quirk that would resolve the > issue. Not finding a fix does not imply closing it as WONTFIX, we need to find a solution for both issues (if they are not the same).

So to be clear: these are server bugs, we are never going to support this image format. Landing https://bugs.webkit.org/show_bug.cgi?id=178758 is probably the only way to move forward here, though it might break these websites in Safari. Alternatively, if you can find a user agent quirk that works, we could use that. But it's certainly not a WebKit bug if we fail to develop a quirk to trick a remote server.

(In reply to Michael Catanzaro from comment #6) > So to be clear: these are server bugs, we are never going to support this > image format. Landing https://bugs.webkit.org/show_bug.cgi?id=178758 is > probably the only way to move forward here, though it might break these > websites in Safari. > > Alternatively, if you can find a user agent quirk that works, we could use > that. But it's certainly not a WebKit bug if we fail to develop a quirk to > trick a remote server. I disagree. At least for some places it is not as easy as it is a server bug and off we go.

I confirm this is because the user agent, once again :-/ dell is sending us jpeg2000 images, using the chrome user agent we receive webp instead.

(In reply to Carlos Garcia Campos from comment #8) > I confirm this is because the user agent, once again :-/ dell is sending us > jpeg2000 images, using the chrome user agent we receive webp instead. OK it's then a matter of quirks isn't it? This is the neverending story...

(In reply to Carlos Garcia Campos from comment #8) > I confirm this is because the user agent, once again :-/ dell is sending us > jpeg2000 images, using the chrome user agent we receive webp instead. I'd use it for baseDomain == dell.com then. Also check kmart.com just in case you have better luck than me, but I think we'll need to write that one off. (In reply to Xabier Rodríguez Calvar from comment #7) > I disagree. At least for some places it is not as easy as it is a server bug > and off we go. How do you propose we fix it if a Chromium user agent quirk does not work (kmart.com)? Only solution I see is to work on removing JPEG 2000 support from Safari and wait until websites notice that they are broken.

(In reply to Michael Catanzaro from comment #10) > (In reply to Carlos Garcia Campos from comment #8) > > I confirm this is because the user agent, once again :-/ dell is sending us > > jpeg2000 images, using the chrome user agent we receive webp instead. > > I'd use it for baseDomain == dell.com then. But it also happens to me in dell.es, and I assume in dell.* too. > Also check kmart.com just in case you have better luck than me, but I think > we'll need to write that one off. Did you remove the disk cache after changing the user agent and trying again? > (In reply to Xabier Rodríguez Calvar from comment #7) > > I disagree. At least for some places it is not as easy as it is a server bug > > and off we go. > > How do you propose we fix it if a Chromium user agent quirk does not work > (kmart.com)? Only solution I see is to work on removing JPEG 2000 support > from Safari and wait until websites notice that they are broken. I'll check why changing the user agent doesn't work in makrt first.

I don't see any broken image in kmart.com

(In reply to Carlos Garcia Campos from comment #11) > Did you remove the disk cache after changing the user agent and trying > again? No, I probably should have. (In reply to Carlos Garcia Campos from comment #12) > I don't see any broken image in kmart.com Looks like Kmart fixed it quite recently. That's nice.

See https://bugs.webkit.org/show_bug.cgi?id=178758#c10 for our latest discovery regarding dell.com. We're simply going to need to add support for JPEG 2000 images. This will affect all ports except Apple ports, because Apple ports do not use WebKit's image decoders.

*** Bug 180995 has been marked as a duplicate of this bug. ***

Ikea webpages (www.ikea.*) are affected by this. Quite annoying since images there are quite important ... Also, Deutsche Bahn (www.bahn.de)

Created attachment 359745 [details] Patch

Attachment 359745 [details] did not pass style-queue: ERROR: Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:39: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:41: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 3 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 359745 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359745&action=review > Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:57 > + *r = std::max(0, std::min(upb, y + static_cast<int>(1.402 * static_cast<float>(cr)))); > + *g = std::max(0, std::min(upb, y - static_cast<int>(0.344 * static_cast<float>(cb) + 0.714 * static_cast<float>(cr)))); > + *b = std::max(0, std::min(upb, y + static_cast<int>(1.772 * static_cast<float>(cb)))); Why not use matrix coefficients wholly, like it's done in the color_esycc_to_rgb() function in openjpeg? Instead of std::min/std::max, you could consider using clampTo<>(), but keeping all of calculation in float. So for r value, you'd have: r = static_cast<int>(clampTo<float>(y - 0.0000368 + 1.40199 * cr, 0, upb)); > Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:68 > + size_t maxw = static_cast<size_t>(img->comps[0].w); > + size_t maxh = static_cast<size_t>(img->comps[0].h); > + size_t max = maxw * maxh; Any way to impose sane limits on these sizes, and/or to protect from overflow?

Comment on attachment 359745 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359745&action=review >> Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:57 >> + *b = std::max(0, std::min(upb, y + static_cast<int>(1.772 * static_cast<float>(cb)))); > > Why not use matrix coefficients wholly, like it's done in the color_esycc_to_rgb() function in openjpeg? > > Instead of std::min/std::max, you could consider using clampTo<>(), but keeping all of calculation in float. > So for r value, you'd have: > > r = static_cast<int>(clampTo<float>(y - 0.0000368 + 1.40199 * cr, 0, upb)); I have no idea, this code is copied from OpenJPEG. I just used min/max instead of the ifs in the original code. I'll use clamp as you suggest. >> Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:68 >> + size_t max = maxw * maxh; > > Any way to impose sane limits on these sizes, and/or to protect from overflow? Ditto. I guess we could use CheckedArithmetic.

Comment on attachment 359745 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359745&action=review >>> Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:57 >>> + *b = std::max(0, std::min(upb, y + static_cast<int>(1.772 * static_cast<float>(cb)))); >> >> Why not use matrix coefficients wholly, like it's done in the color_esycc_to_rgb() function in openjpeg? >> >> Instead of std::min/std::max, you could consider using clampTo<>(), but keeping all of calculation in float. >> So for r value, you'd have: >> >> r = static_cast<int>(clampTo<float>(y - 0.0000368 + 1.40199 * cr, 0, upb)); > > I have no idea, this code is copied from OpenJPEG. I just used min/max instead of the ifs in the original code. I'll use clamp as you suggest. color_esycc_to_rgb() is used when color space is OPJ_CLRSPC_EYCC. We don't support that one (yet).

Created attachment 359870 [details] Patch

Attachment 359870 [details] did not pass style-queue: ERROR: Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:39: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:41: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 3 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.

We need OpenJPEG 2.2.0, not available in debian stable, so we'll need to build it in jhbuild.

Created attachment 359873 [details] Patch

Attachment 359873 [details] did not pass style-queue: ERROR: Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:39: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.cpp:41: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/platform/image-decoders/jpeg2000/JPEG2000ImageDecoder.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 3 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 359873 [details] Patch Attachment 359873 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/10857660 New failing tests: imported/w3c/web-platform-tests/css/css-grid/abspos/orthogonal-positioned-grid-descendants-015.html imported/w3c/web-platform-tests/css/css-grid/abspos/orthogonal-positioned-grid-descendants-013.html

Created attachment 359881 [details] Archive of layout-test-results from ews106 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6

Amazing!

Committed r240428: <https://trac.webkit.org/changeset/240428>

<rdar://problem/47509724>