Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add <img> decoding="" attribute #3221

Merged
merged 4 commits into from
Dec 5, 2017
Merged

Add <img> decoding="" attribute #3221

merged 4 commits into from
Dec 5, 2017

Conversation

vmpstr
Copy link
Member

@vmpstr vmpstr commented Nov 10, 2017

This pull introduces a decoding attribute to the img tag. The attribute indicates a decoding hint to
the user agent. That is, this hint aids the user agent in deciding how to process and decode the image
before rasterizing it. Possible values are as follows:

  • sync - prefer to decode the image synchronously for atomic presentation with other content
  • async - prefer to decode the image asynchronously to reduce delay in presenting other content
  • auto - default mode, which indicates no preference for the decoding mode.

Closes #1920


/embedded-content.html ( diff )
/images.html ( diff )
/index.html ( diff )
/indices.html ( diff )

@vmpstr
Copy link
Member Author

vmpstr commented Nov 10, 2017

/cc: @chrishtr @smfr @domenic @annevk

This attribute has three states (sync/async/auto) as per discussion on #1920. Also, the name of the attribute is decodingMode to eliminate amiguity with the decode() function.

source Outdated
@@ -25473,6 +25475,10 @@ interface <dfn>HTMLImageElement</dfn> : <span>HTMLElement</span> {
cannot process images or who have image loading disabled (i.e. it is the <code>img</code>
element's <span>fallback content</span>).</p>

<p>Image data can appear in encoded form in order to reduce file size.
<dfn data-x="img-decoding-process">Decoding</dfn> is a process which converts image's media data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"converts an image's"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"is the process"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

source Outdated
<tbody>
<tr>
<td><dfn><code data-x="attr-img-decodingmode-sync">sync</code></dfn>
<td>Inidicates a preference to <span data-x="img-decoding-process">decode</span> this image
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sp: indicates

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

source Outdated
<tr>
<td><dfn><code data-x="attr-img-decodingmode-sync">sync</code></dfn>
<td>Inidicates a preference to <span data-x="img-decoding-process">decode</span> this image
synchronosly for atomic presentation with other content.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sp: synchronous

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't say what "synchronous" is relative to. I think the spec needs to say something to the effect of "synchronous" meaning that the image will get painted immediately after the image data is available (i.e. the load event firing).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, any ideas for how to make this clearer would be appreciated, perhaps by integrating with the img processing model. (Although that seems to have very little about actually presenting on the screen.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a section on Decoding images (as @domenic recommended). I appreciate any suggestions on how to phrase that section better.

source Outdated
asynchronously to avoid delaying presentation of other content.
<tr>
<td><dfn><code data-x="attr-img-decodingmode-auto">auto</code></dfn>
<td>Indicates no preference in decoding mode, which also servces as the default value if the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sp: serves

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

source Outdated
@@ -25473,6 +25475,10 @@ interface <dfn>HTMLImageElement</dfn> : <span>HTMLElement</span> {
cannot process images or who have image loading disabled (i.e. it is the <code>img</code>
element's <span>fallback content</span>).</p>

<p>Image data can appear in encoded form in order to reduce file size.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"can appear" is weird (it sounds like it's about putting something on the screen). Maybe "Image data is usually encoded to reduce file size".

source Outdated
@@ -25473,6 +25475,10 @@ interface <dfn>HTMLImageElement</dfn> : <span>HTMLElement</span> {
cannot process images or who have image loading disabled (i.e. it is the <code>img</code>
element's <span>fallback content</span>).</p>

<p>Image data can appear in encoded form in order to reduce file size.
<dfn data-x="img-decoding-process">Decoding</dfn> is a process which converts image's media data
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"is the process"

source Outdated
<tr>
<td><dfn><code data-x="attr-img-decodingmode-sync">sync</code></dfn>
<td>Inidicates a preference to <span data-x="img-decoding-process">decode</span> this image
synchronosly for atomic presentation with other content.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't say what "synchronous" is relative to. I think the spec needs to say something to the effect of "synchronous" meaning that the image will get painted immediately after the image data is available (i.e. the load event firing).

source Outdated
<dt><var>image</var> . <code subdfn data-x="dom-img-decode">decode</code>()</dt>

<dd>

<p>Images usually exist in some encoded form; user agents need to decode them into raw pixels
<p>Images usually exist in some encoded form; user agents need to
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd that this wording is similar to, but not exactly the same as the wording above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've simplified this part and referenced the decoding images section.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start :)

source Outdated
@@ -25511,6 +25517,10 @@ interface <dfn>HTMLImageElement</dfn> : <span>HTMLElement</span> {
<span>referrer policy attribute</span>. Its purpose is to set the <span>referrer policy</span>
used when <span data-x="concept-fetch">fetching</span> the image. <ref spec=REFERRERPOLICY></p>

<p>The <dfn data-x="attr-img-decodingmode"><code>decodingmode</code></dfn> attribute which, when
set from a list of possible <span>image decoding mode hints</span>, indicates the preferred
decoding mode to use for this image.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to match other enumerated content attribute definitions more closely. In particular, it needs to state the states, keywords, invalid value default, and missing value default. See for example https://html.spec.whatwg.org/#the-dir-attribute or https://html.spec.whatwg.org/#attr-track-kind. You have some of this below, but it belongs with the content attribute, not the IDL attribute.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. Done.

source Outdated
@@ -25473,6 +25475,10 @@ interface <dfn>HTMLImageElement</dfn> : <span>HTMLElement</span> {
cannot process images or who have image loading disabled (i.e. it is the <code>img</code>
element's <span>fallback content</span>).</p>

<p>Image data can appear in encoded form in order to reduce file size.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted below, this paragraph should be inside the image processing model section.

source Outdated
<code data-x="attr-img-decodingmode-sync">sync</code> or
<code data-x="attr-img-decodingmode-async">async</code>. If the value specified is
<code data-x="attr-img-decodingmode-auto">auto</code>, then the user-agent is free to choose any
decoding behavior.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be located here; instead it should be in the normative section on the image processing model, along with the definition of decoding. I would create a new section, "Decoding images", before "Updating the image data", which explains that decoding is usually part of how the user agent presents the image, but can happen at various times depending on user agent heuristics, the value of the decodingmode attribute, and the decode() method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, done! I wasn't sure if I should move the decode() description there as well listing all of the steps that take place when decode is invoked. WDYT?

source Outdated

<dd>

<p>Returns the image's preferred <span data-x="dom-img-decodingMode">decoding mode</span>.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dom-img-decodingMode is just an IDL attribute. You should instead link to "image decoding mode hint".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

source Outdated
<tr>
<td><dfn><code data-x="attr-img-decodingmode-sync">sync</code></dfn>
<td>Inidicates a preference to <span data-x="img-decoding-process">decode</span> this image
synchronosly for atomic presentation with other content.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, any ideas for how to make this clearer would be appreciated, perhaps by integrating with the img processing model. (Although that seems to have very little about actually presenting on the screen.)

source Outdated
<tr>
<td><dfn><code data-x="attr-img-decodingmode-async">async</code></dfn>
<td>Indicates a preference to <span data-x="img-decoding-process">decode</span> this image
asynchronously to avoid delaying presentation of other content.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using "asynchronously" (and "synchronously") is kind of begging the question, here. Can we do better? Either describing the implementation strategy, or the user-facing effects, or both?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some description in a separate section.

source Outdated
<th> <code data-x="">decodingmode</code>
<td> <code data-x="attr-img-decodingmode">img</code>;
<td> Decoding mode hint to use when processing this image for presentation.
<td> Comma-separated list of <span data-x="image decoding mode hints">image decoding mode hints</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a comma-separated list...

Following the example of other enumerated attributes, this should be a list of strings each of which link to the keyword definition. See e.g. the dir attribute in this same table.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

source Outdated
@@ -117474,6 +117524,11 @@ interface <dfn>External</dfn> {
<span>valid non-negative integer</span>, or
<span>valid duration string</span>
<tr>
<th> <code data-x="">decodingmode</code>
<td> <code data-x="attr-img-decodingmode">img</code>;
<td> Decoding mode hint to use when processing this image for presentation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No period. Probably something like "Decoding mode hint for the image" where "Decoding mode hint" links to the image decoding mode hints concept.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member Author

@vmpstr vmpstr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I've renamed the attribute to "decoding" as per conversation in "decode" attribute on <img> #1920. I think it makes sense, but that's an easy thing to change if we change our mind.

  • I've also added Decoding images section as a place to describe some of the processes that happen in the user agent and to define what synchronous and asynchronous mean.

source Outdated
@@ -25473,6 +25475,10 @@ interface <dfn>HTMLImageElement</dfn> : <span>HTMLElement</span> {
cannot process images or who have image loading disabled (i.e. it is the <code>img</code>
element's <span>fallback content</span>).</p>

<p>Image data can appear in encoded form in order to reduce file size.
<dfn data-x="img-decoding-process">Decoding</dfn> is a process which converts image's media data
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

source Outdated
@@ -25511,6 +25517,10 @@ interface <dfn>HTMLImageElement</dfn> : <span>HTMLElement</span> {
<span>referrer policy attribute</span>. Its purpose is to set the <span>referrer policy</span>
used when <span data-x="concept-fetch">fetching</span> the image. <ref spec=REFERRERPOLICY></p>

<p>The <dfn data-x="attr-img-decodingmode"><code>decodingmode</code></dfn> attribute which, when
set from a list of possible <span>image decoding mode hints</span>, indicates the preferred
decoding mode to use for this image.</p>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. Done.

source Outdated
<tbody>
<tr>
<td><dfn><code data-x="attr-img-decodingmode-sync">sync</code></dfn>
<td>Inidicates a preference to <span data-x="img-decoding-process">decode</span> this image
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

source Outdated
<tr>
<td><dfn><code data-x="attr-img-decodingmode-sync">sync</code></dfn>
<td>Inidicates a preference to <span data-x="img-decoding-process">decode</span> this image
synchronosly for atomic presentation with other content.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a section on Decoding images (as @domenic recommended). I appreciate any suggestions on how to phrase that section better.

source Outdated
<tr>
<td><dfn><code data-x="attr-img-decodingmode-async">async</code></dfn>
<td>Indicates a preference to <span data-x="img-decoding-process">decode</span> this image
asynchronously to avoid delaying presentation of other content.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some description in a separate section.

source Outdated
<code data-x="attr-img-decodingmode-sync">sync</code> or
<code data-x="attr-img-decodingmode-async">async</code>. If the value specified is
<code data-x="attr-img-decodingmode-auto">auto</code>, then the user-agent is free to choose any
decoding behavior.</p>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, done! I wasn't sure if I should move the decode() description there as well listing all of the steps that take place when decode is invoked. WDYT?

source Outdated

<dd>

<p>Returns the image's preferred <span data-x="dom-img-decodingMode">decoding mode</span>.</p>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

source Outdated
@@ -117474,6 +117524,11 @@ interface <dfn>External</dfn> {
<span>valid non-negative integer</span>, or
<span>valid duration string</span>
<tr>
<th> <code data-x="">decodingmode</code>
<td> <code data-x="attr-img-decodingmode">img</code>;
<td> Decoding mode hint to use when processing this image for presentation.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

source Outdated
<dt><var>image</var> . <code subdfn data-x="dom-img-decode">decode</code>()</dt>

<dd>

<p>Images usually exist in some encoded form; user agents need to decode them into raw pixels
<p>Images usually exist in some encoded form; user agents need to
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've simplified this part and referenced the decoding images section.

source Outdated
<th> <code data-x="">decodingmode</code>
<td> <code data-x="attr-img-decodingmode">img</code>;
<td> Decoding mode hint to use when processing this image for presentation.
<td> Comma-separated list of <span data-x="image decoding mode hints">image decoding mode hints</span>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@chrishtr
Copy link
Contributor

Latest patchset LGTM.

@vmpstr vmpstr changed the title Add img.decodingMode attribute Add img.decoding attribute Nov 14, 2017
source Outdated
attribute is not specified.
</table>

<p class="note">The user agent <!--non-normative-->should respect the value of the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like a non-normative note. You know something's going wrong if you're overriding the linter...

I'll fix.

source Outdated
<p class="note">If the <code data-x="attr-img-decoding">decoding</code> attribute is missing, its
value is not specified, or the value specified is not one of the above listed values, then the
effect is equivalent to specifying the <code data-x="attr-img-decoding">decoding</code>
attribute with <code data-x="attr-img-decoding-auto">auto</code> value.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to normatively specify this using invalid and missing value defaults. I will fix.

@domenic
Copy link
Member

domenic commented Nov 16, 2017

I pushed a commit with a few more tweaks. This now looks good to me :).

As for tests, the only ones I can think of are updating https://github.com/w3c/web-platform-tests/blob/11abef74e5f06c23176ab2f492c71c9a33250742/html/dom/elements-embedded.js#L3 . If we can get those updated, either as a WPT PR or via Chromium's auto-export, I'm ready to merge!

@domenic domenic added addition/proposal New features or enhancements topic: img needs tests Moving the issue forward requires someone to write tests labels Nov 16, 2017
@domenic
Copy link
Member

domenic commented Nov 16, 2017

Hmm I suppose we should get Apple's sign-off on the name. Let's have that discussion back in #1920, I guess? Marking "do not merge yet" just in case.

@domenic domenic added the do not merge yet Pull request must not be merged per rationale in comment label Nov 16, 2017
@domenic domenic removed the do not merge yet Pull request must not be merged per rationale in comment label Nov 16, 2017
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there's again some unrelated commits in this PR. (I suspect they might all be related to the little incident we had on master yesterday...)

<tbody>
<tr>
<td><dfn><code data-x="attr-img-decoding-sync">sync</code></dfn>
<td><dfn data-x="attr-img-decoding-sync-state">Sync</dfn>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we spell these out as "Synchronous" and "Asynchronous"? We typically don't abbreviate states I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the whole states vs. keywords distinction already a bit hard to deal with, so I try for new stuff to keep them as close as possible. But I don't have a principled suggestion going forward, so happy to change it if you prefer.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell you haven't updated the Index for the img element.

@domenic
Copy link
Member

domenic commented Nov 17, 2017

Fixed the extra commit; that was my bad. All of yesterday I had an extra commit floating around on master messing up all my rebases. Also updated the element index. (It'd be so nice if those were generated...)

@@ -115885,6 +115963,7 @@ interface <dfn>External</dfn> {
<code data-x="attr-img-ismap">ismap</code>;
<code data-x="attr-dim-width">width</code>;
<code data-x="attr-dim-height">height</code>;
<code data-x="attr-img-decoding">decoding</code>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this should come after referrerpolicy if we want to retain the order we started in the img element section.

@annevk annevk added the do not merge yet Pull request must not be merged per rationale in comment label Nov 19, 2017
@annevk
Copy link
Member

annevk commented Nov 19, 2017

Adding "do not merge yet" back as the discussion in the corresponding issue hasn't quite completed. Note that this is also still blocked on new web-platform-tests tests.

@vmpstr
Copy link
Member Author

vmpstr commented Nov 22, 2017

Adding "do not merge yet" back as the discussion in the corresponding issue hasn't quite completed. Note that this is also still blocked on new web-platform-tests tests.

As an update, I think the discussion on #1920 has concluded with the consensus that we should name the attribute decoding (as is in the latest patchset here). I'm adding web platform tests in here: web-platform-tests/wpt#8382 which is currently pending review.

@domenic
Copy link
Member

domenic commented Nov 27, 2017

Tests are arriving via https://chromium-review.googlesource.com/c/chromium/src/+/770106.

@annevk, @dbaron, do you feel that the concerns raised that you linked to have been addressed or no? They seem addressed to me, but I want to be sure before merging...

@annevk
Copy link
Member

annevk commented Dec 5, 2017

@vmpstr would you be willing to file browser bugs? I'm guessing that nobody but Chrome has this recorded thus far.

@annevk annevk merged commit 05525c8 into whatwg:master Dec 5, 2017
@domenic domenic changed the title Add img.decoding attribute Add <img> decoding="" attribute Dec 5, 2017
@vmpstr
Copy link
Member Author

vmpstr commented Dec 5, 2017

@vmpstr would you be willing to file browser bugs? I'm guessing that nobody but Chrome has this recorded thus far.

Yes, I'll file some bugs.

@smfr
Copy link

smfr commented Dec 5, 2017

@vmpstr
Copy link
Member Author

vmpstr commented Dec 5, 2017

@vmpstr
Copy link
Member Author

vmpstr commented Dec 5, 2017

@vmpstr
Copy link
Member Author

vmpstr commented Dec 5, 2017

Chrome: crbug.com/772470

@annevk
Copy link
Member

annevk commented Dec 6, 2017

Thanks! Kudos on getting this done. (By the way, are you mentioned in the Acknowledgments section already?)

@vmpstr
Copy link
Member Author

vmpstr commented Dec 6, 2017

By the way, are you mentioned in the Acknowledgments section already?

Yeah I'm there :) Thanks!

hubot pushed a commit to WebKit/WebKit-http that referenced this pull request Dec 7, 2017
https://bugs.webkit.org/show_bug.cgi?id=179432

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2017-12-06
Reviewed by Darin Adler.

Source/WebCore:

Support the HTMLElementImage decoding attribute to allow controlling the
decoding of an image as per:
        whatwg/html#3221
        https://whatpr.org/html/3221/images.html#decoding-images

Tests: fast/images/decode-decoding-attribute-async-large-image.html
       fast/images/decoding-attribute-async-small-image.html
       fast/images/decoding-attribute-dynamic-async-small-image.html
       fast/images/decoding-attribute-sync-large-image.html

* html/HTMLAttributeNames.in:
* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::decodingMode const):
* html/HTMLImageElement.h:
* html/HTMLImageElement.idl:
* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::draw):
(WebCore::BitmapImage::internalStartAnimation):
(WebCore::BitmapImage::advanceAnimation):
(WebCore::BitmapImage::internalAdvanceAnimation):
* platform/graphics/DecodingOptions.h:
(WebCore::DecodingOptions::DecodingOptions):
(WebCore::DecodingOptions::isAuto const):
(WebCore::DecodingOptions::isAsynchronousCompatibleWith const):
(WebCore::DecodingOptions::isNone const): Deleted.
* platform/graphics/ImageDecoder.h:
* platform/graphics/ImageFrame.cpp:
(WebCore::ImageFrame::clearImage):
* platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::frameAtIndexCacheIfNeeded):
* platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.h:
* platform/graphics/cg/ImageDecoderCG.h:
* platform/graphics/win/ImageDecoderDirect2D.h:
* platform/image-decoders/ScalableImageDecoder.h:
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::decodingModeForImageDraw const):
The element attributes and the document settings should be checked before
checking our heuristics. And since the "decoding" attribute is per an image
and isLargeImageAsyncDecodingEnabledForTesting() is a global setting, the
decoding attribute should be checked first.

LayoutTests:

* fast/images/async-attribute-with-small-image-expected.html: Removed.
* fast/images/async-attribute-with-small-image.html: Removed.
* fast/images/decode-decoding-attribute-async-large-image-expected.html: Added.
* fast/images/decode-decoding-attribute-async-large-image.html: Added.
* fast/images/decoding-attribute-async-small-image-expected.html: Added.
* fast/images/decoding-attribute-async-small-image.html: Added.
* fast/images/decoding-attribute-dynamic-async-small-image-expected.html: Added.
* fast/images/decoding-attribute-dynamic-async-small-image.html: Added.
* fast/images/decoding-attribute-sync-large-image-expected.html: Added.
* fast/images/decoding-attribute-sync-large-image.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@225616 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@foolip
Copy link
Member

foolip commented Dec 14, 2017

@tobie, https://whatpr.org/html/3221/8b055c0...3f6a0fe/images.html is b0rked, can you investigate?

@tobie
Copy link
Contributor

tobie commented Dec 14, 2017

@foolip it's an issue with the <pattern> element which isn't recognized by Tidy on htmldiff. I emailed @dontcallmedom to see if he could special-case it on his side.

@tobie
Copy link
Contributor

tobie commented Dec 14, 2017

Fixed on the htmldiff side by @dontcallmedom. Now restarting the build and cleaning the caches on the pr-preview side.

@tobie
Copy link
Contributor

tobie commented Dec 14, 2017

And now fixed.

@foolip
Copy link
Member

foolip commented Dec 14, 2017

Awesome, thanks!

aosmond added a commit to aosmond/gecko-dev that referenced this pull request Feb 21, 2018
aosmond added a commit to aosmond/gecko-dev that referenced this pull request Jul 3, 2018
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 8, 2018
…z,tnikkel

This adds support for HTMLImageElement's decoding attribute, as
described by:

whatwg/html#3221
https://whatpr.org/html/3221/images.html#decoding-images

It also exposes the same attribute on SVGImageElement, just as Blink has
chosen to do so.
jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Aug 8, 2018
…z,tnikkel

This adds support for HTMLImageElement's decoding attribute, as
described by:

whatwg/html#3221
https://whatpr.org/html/3221/images.html#decoding-images

It also exposes the same attribute on SVGImageElement, just as Blink has
chosen to do so.
aosmond added a commit to aosmond/gecko-dev that referenced this pull request Aug 30, 2018
…z,tnikkel

This adds support for HTMLImageElement's decoding attribute, as
described by:

whatwg/html#3221
https://whatpr.org/html/3221/images.html#decoding-images

It also exposes the same attribute on SVGImageElement, just as Blink has
chosen to do so.
aosmond added a commit to aosmond/gecko-dev that referenced this pull request Aug 30, 2018
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
…z,tnikkel

This adds support for HTMLImageElement's decoding attribute, as
described by:

whatwg/html#3221
https://whatpr.org/html/3221/images.html#decoding-images

It also exposes the same attribute on SVGImageElement, just as Blink has
chosen to do so.

UltraBlame original commit: 630049a9ac3b24ec0a141669937df372373a5d6f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
…z,tnikkel

This adds support for HTMLImageElement's decoding attribute, as
described by:

whatwg/html#3221
https://whatpr.org/html/3221/images.html#decoding-images

It also exposes the same attribute on SVGImageElement, just as Blink has
chosen to do so.

UltraBlame original commit: 630049a9ac3b24ec0a141669937df372373a5d6f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
…z,tnikkel

This adds support for HTMLImageElement's decoding attribute, as
described by:

whatwg/html#3221
https://whatpr.org/html/3221/images.html#decoding-images

It also exposes the same attribute on SVGImageElement, just as Blink has
chosen to do so.

UltraBlame original commit: 630049a9ac3b24ec0a141669937df372373a5d6f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements do not merge yet Pull request must not be merged per rationale in comment topic: img
Development

Successfully merging this pull request may close these issues.

None yet

8 participants