Closed Bug 1680387 Opened 3 years ago Closed 3 years ago

Images should be able to declare their own intrinsic resolution/size (EXIF density not honored)

Categories

(Core :: Layout: Images, Video, and HTML Frames, enhancement)

Firefox 85
enhancement

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: e, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.0 Safari/605.1.15

Steps to reproduce:

Run any of these tests:
https://wpt.fyi/results/density-size-correction?label=master&label=experimental&aligned&q=density

Actual results:

Firefox does not respect the EXIF information in the JPEG, and fails the tests.

Expected results:

Firefox should set the image's intrinsic size based on EXIF information, and pass the tests.

See the spec change, discussion, and links to the commits that landed this in Chromium and WebKit, here: https://github.com/whatwg/html/pull/5574

Component: Untriaged → Layout: Images, Video, and HTML Frames
Product: Firefox → Core

Andrew / Tim, this seems more of an ImageLib thing than a layout thing right?

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(tnikkel)
Flags: needinfo?(aosmond)
Summary: Images should be able to declare their own intrinsic resolution/size → Images should be able to declare their own intrinsic resolution/size (EXIF density not honored)
Assignee: nobody → emilio
Flags: needinfo?(tnikkel)
Flags: needinfo?(aosmond)

This should be mostly straight-forward, since we have code for this
anyways for image-set() and srcset.

The only thing is that we were using floats for resolution, but since
EXIF allows you to scale each axis separately, we now need to pass an
image::Resolution instead.

The main outstanding issue is the spec comment mentioned in the previous
patch, about what happens if you have srcset/image-set and the image
density specified together. For now I've implemented what the
image-set() spec says, but this is subject to change before shipping of
course.

Blocks: 1707697
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c1c5e1f901b
Read and expose EXIF image resolution data. r=tnikkel,aosmond
https://hg.mozilla.org/integration/autoland/rev/51531850a9a2
Apply intrinsic image resolution as appropriate in layout/style/dom, and update test expectations. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/65616921e520
Fix interaction with src-set() / image-set(), and enable the feature by default. r=tnikkel
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/28811 for changes under testing/web-platform/tests

Backed out for causing browser chrome failures on browser_bug592641.js.

Push with failures

Failure log

Backout link

Flags: needinfo?(emilio)
Upstream PR was closed without merging

Also matches other browsers, and fixes the test that got me backed out,
since it has a huge EXIF resolution value.

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9317cd872430
Read and expose EXIF image resolution data. r=tnikkel,aosmond
https://hg.mozilla.org/integration/autoland/rev/4f74cdfb4fdc
Apply intrinsic image resolution as appropriate in layout/style/dom, and update test expectations. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/2ac7c9cd4629
Fix interaction with src-set() / image-set(), and enable the feature by default. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/82461e5bf8ae
Sanity-check exif resolution with other exif metadata. r=tnikkel
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/09481b273d7b
For some reason exif_resolution.jpg landed empty?
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/021e5ade2475
Restore some test annotations that are still failing.
Blocks: 1709622
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/f573ec57b561
Indent correctly one fuzzy annotation.
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.