Bug 123832 – Use srcset's pixel density to determine intrinsic size

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Yoav Weiss]

Use srcset's pixel density to determine intrinsic size

[Yoav Weiss]

Created attachment 216088 [details]
Patch

[Yoav Weiss]

The patch is a port of a similar Blink patch: https://codereview.chromium.org/25105004

According to the spec "When an img element has a current pixel density that is not 1.0, the element's image data must be treated as if its resolution, in device pixels per CSS pixels, was the current pixel density."

[Yoav Weiss]

Created attachment 216137 [details]
Patch

[EFL EWS Bot]

Comment on attachment 216137 [details]
Patch

Attachment 216137 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/21748014

[EFL EWS Bot]

Comment on attachment 216137 [details]
Patch

Attachment 216137 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/21818007

[kov's GTK+ EWS bot]

Comment on attachment 216137 [details]
Patch

Attachment 216137 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/21308119

[Build Bot]

Comment on attachment 216137 [details]
Patch

Attachment 216137 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/21778009

[Build Bot]

Comment on attachment 216137 [details]
Patch

Attachment 216137 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/21858019

[Yoav Weiss]

Created attachment 216141 [details]
Patch

[Build Bot]

Comment on attachment 216141 [details]
Patch

Attachment 216141 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/21858033

New failing tests:
fast/hidpi/image-srcset-simple-2x.html
fast/hidpi/image-srcset-invalid-inputs-except-one.html
fast/hidpi/image-srcset-fraction.html
fast/hidpi/image-srcset-change-dynamically-from-js-2x.html
fast/hidpi/image-srcset-png-canvas.html
fast/hidpi/image-srcset-src-selection-2x.html
fast/hidpi/image-srcset-invalid-inputs-correct-src.html
fast/hidpi/image-srcset-fraction-1.5x.html
fast/hidpi/image-srcset-relative-svg-canvas-2x.html
fast/hidpi/image-srcset-svg-canvas.html

[Build Bot]

Created attachment 216146 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 216141 [details]
Patch

Attachment 216141 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/21808039

New failing tests:
fast/hidpi/image-srcset-simple-2x.html
fast/hidpi/image-srcset-invalid-inputs-except-one.html
fast/hidpi/image-srcset-fraction.html
fast/hidpi/image-srcset-change-dynamically-from-js-2x.html
fast/hidpi/image-srcset-png-canvas.html
fast/hidpi/image-srcset-src-selection-2x.html
fast/hidpi/image-srcset-invalid-inputs-correct-src.html
fast/hidpi/image-srcset-fraction-1.5x.html
fast/hidpi/image-srcset-relative-svg-canvas-2x.html
fast/hidpi/image-srcset-svg-canvas.html

[Build Bot]

Created attachment 216152 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Yoav Weiss]

Created attachment 216996 [details]
Patch

[Build Bot]

Comment on attachment 216996 [details]
Patch

Attachment 216996 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/23688089

New failing tests:
fast/hidpi/image-srcset-change-dynamically-from-js-2x.html
fast/hidpi/image-srcset-png-canvas.html

[Build Bot]

Created attachment 217002 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 216996 [details]
Patch

Attachment 216996 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/23638127

New failing tests:
fast/hidpi/image-srcset-change-dynamically-from-js-2x.html
fast/hidpi/image-srcset-png-canvas.html

[Build Bot]

Created attachment 217005 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 216996 [details]
Patch

Attachment 216996 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/23688120

New failing tests:
fast/hidpi/image-srcset-change-dynamically-from-js-2x.html
fast/hidpi/image-srcset-png-canvas.html

[Build Bot]

Created attachment 217009 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 216996 [details]
Patch

Attachment 216996 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/21718250

New failing tests:
fast/hidpi/image-srcset-change-dynamically-from-js-2x.html
fast/hidpi/image-srcset-png-canvas.html

[Build Bot]

Created attachment 217012 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Yoav Weiss]

Created attachment 217024 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 217024 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=217024&action=review

> Source/WebCore/ChangeLog:10
> +        Reviewed by NOBODY (OOPS!).

This line should appear before the long description but after the bug url.

[Yoav Weiss]

Created attachment 217178 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/15939819>

[Theresa O'Connor]

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

[Yoav Weiss]

Created attachment 222633 [details]
Patch

[Yoav Weiss]

Created attachment 222645 [details]
Patch

[Yoav Weiss]

Created attachment 222659 [details]
Patch

[Dean Jackson]

Comment on attachment 222659 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=222659&action=review

Some small things to fix up.

> Source/WebCore/ChangeLog:39543
>  2013-11-14  Gyuyoung Kim  <[e-mail]>
> -
>          Introduce FILTER_TYPE_CASTS for child filter class

What?

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1209
> +    CachedImage* cachedImage = image->cachedImage();
> +    if (cachedImage) {

I think this should go back to being if (CachedImage* cachedImage = ....)

> LayoutTests/ChangeLog:14
> +        Layout test changes include modifications of existing tests to accomodate the new image dimensions, as well as new tests for this
> +        specific functionality

Nit, missing .

> LayoutTests/ChangeLog:27
> +        * fast/hidpi/image-srcset-invalid-inputs-expected.txt: Added.

I'm confused here - I don't see the actual test, just the expected results. And it's a DRT dump, so probably should be in platform.

> LayoutTests/TestExpectations:62
> +webkit.org/b/124342 fast/hidpi/image-srcset-svg-canvas.html [ Skip ]
> +webkit.org/b/124342 fast/hidpi/image-srcset-svg-canvas-2x.html [ Skip ]
> +webkit.org/b/124349 fast/hidpi/image-srcset-relative-svg-canvas-2x.html [ Skip ]
> +webkit.org/b/124349 fast/hidpi/image-srcset-relative-svg-canvas.html [ Skip ]
> +

Why are you skipping these? Your changelog doesn't mention it. Why skip tests that you're adding in this patch? It might be better to check in failing results.

> LayoutTests/fast/hidpi/image-srcset-invalid-inputs-expected.txt:11
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x600
> +  RenderBlock {HTML} at (0,0) size 800x600
> +    RenderBody {BODY} at (8,8) size 784x584
> +      RenderBlock {DIV} at (0,0) size 784x17
> +        RenderText {#text} at (0,0) size 779x17
> +          text run at (0,0) width 779: "This test passes if this img tag below is empty and displays nothing. It ensures that the srcset attribute supports invalid inputs"
> +      RenderBlock (anonymous) at (0,17) size 784x100
> +        RenderImage {IMG} at (0,0) size 100x100
> +        RenderText {#text} at (0,0) size 0x0

See above for comment.

[Yoav Weiss]

Created attachment 223120 [details]
Patch

[Build Bot]

Comment on attachment 223120 [details]
Patch

Attachment 223120 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5695200588464128

New failing tests:
fast/hidpi/image-srcset-invalid-inputs.html

[Build Bot]

Created attachment 223124 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Dean Jackson]

Comment on attachment 223120 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=223120&action=review

Go for it, once you've fixed the failure. Thanks Yoav!

> LayoutTests/TestExpectations:64
> +webkit.org/b/124342 fast/hidpi/image-srcset-svg-canvas.html [ Skip ]
> +webkit.org/b/124342 fast/hidpi/image-srcset-svg-canvas-2x.html [ Skip ]
> +webkit.org/b/124349 fast/hidpi/image-srcset-relative-svg-canvas-2x.html [ Skip ]
> +webkit.org/b/124349 fast/hidpi/image-srcset-relative-svg-canvas.html [ Skip ]

Maybe we should mark these as failing rather than skipping. That way we won't forget them when we fix the other bugs.

[Build Bot]

Comment on attachment 223120 [details]
Patch

Attachment 223120 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5398243663937536

New failing tests:
fast/hidpi/image-srcset-invalid-inputs.html

[Build Bot]

Created attachment 223136 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Yoav Weiss]

Created attachment 223146 [details]
Patch

[Yoav Weiss]

Created attachment 223157 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 223157 [details]
Patch

Clearing flags on attachment: 223157

Committed r163415: <http://trac.webkit.org/changeset/163415>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.