Closed Bug 1680387 Opened 4 years ago Closed 4 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.

Attachment

General

Created:
Updated:
Size: