currentSrc and data URIs #223

Closed
yoavweiss opened this Issue Jun 25, 2014 · 8 comments

Projects

None yet

4 participants

@yoavweiss
Member

When setting a srcset attribute with data URI, should the currentSrc value be set in a synchronous manner?
If so, such a technique can be used for future feature detection once we add further descriptors to srcset. On the other hand, such feature detection would require some assumptions regarding the srcset selection algo (which we tell devs they shouldn't be making any).

@marcoscaceres
Member

If currentSrc just reflects the chosen src (irrespective of URL scheme), then it should be fine to do it sync IMO.

@zcorpan
zcorpan commented Jun 29, 2014

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3066 - seems data: URL images load sync in blink/gecko at least. Will need to fix "update the image data" to do that. (I guess blob: should still be async?)

@zcorpan
zcorpan commented Jan 13, 2015

It's not sync in Gecko anymore. I don't know when/why this changed.

It's not sync in Safari 8 (on Mac OS X 10.10.1).

It is still sync in Blink.

It's not sync in IE11.

Seems like the trend is towards async loading of <img src=data:...>. This happens to be what the spec requires already.

Another reason to avoid sync loading is that it would be blocking while decoding the image which can take some time for very big images.

@zcorpan zcorpan closed this Jan 13, 2015
@progers
progers commented Jul 28, 2015

Does a spec require synchronous loads? The html5 spec seems to imply synchronous loads are cool:
"A user agent that obtains images immediately must synchronously update the image data of an img element..."

Safari will synchronously load images (datauri or not, so long as they are cached) on refresh:
http://jsfiddle.net/jfe3r3nb/4 (refresh twice)

For this reason, sites must have code to support synchronous loads already.

@yoavweiss
Member

Does a spec require synchronous loads? The html5 spec seems to imply synchronous loads are cool:
"A user agent that obtains images immediately must synchronously update the image data of an img element..."

update the image data runs synchronously, but inside it, image load is async, unless the image is in cache (see 5.3).
With that said, we could change the spec if we think it's beneficial.

Safari will synchronously load images (datauri or not, so long as they are cached) on refresh:
http://jsfiddle.net/jfe3r3nb/4 (refresh twice)

Yeah, if the image is in memory cache, it will be sync loaded (and I believe sites rely on that behavior, so we cannot remove it).

The main problematic scenario here is the Flash Of Wrong Image™, which admittedly, can also happen with cached images (although not with a broken image). Here's a test case showing that (reported by @afarkas)

@yoavweiss
Member

As a reference, we've been discussing turning data URI resource load to async in Blink

@yoavweiss yoavweiss reopened this Jul 28, 2015
@dstockwell dstockwell pushed a commit to dstockwell/chromium that referenced this issue Dec 7, 2015
@yoavweiss yoavweiss + Commit bot Load data URI images in an async way according to spec
According to ResponsiveImagesCG/picture-element#223 (comment)
we seem to be the only browser that loads data URIs immediately (as if they were cached).
It was introduced as part of the move to async image loading: https://codereview.chromium.org/200923002

This is now causing issues: https://code.google.com/p/chromium/issues/detail?id=469131

This CL changes this behavior to be spec compliant and load data URIs asynchronously, like other resources.

This is bringing https://codereview.chromium.org/1256233002/ to the Chromium repo.

BUG=514206

Review URL: https://codereview.chromium.org/1492303003

Cr-Commit-Position: refs/heads/master@{#363444}
91aa6fb
@yoavweiss
Member

dataURIs are now loaded async in Blink, which according to #223 (comment) they are async everywhere. Closing

@yoavweiss yoavweiss closed this Dec 7, 2015
@yoavweiss
Member

Correction: They may not be async in WebKit (as image loading is still sync there in general), but that would hopefully soon change.

@dstockwell dstockwell pushed a commit to dstockwell/chromium that referenced this issue Dec 16, 2015
@yoavweiss yoavweiss + Commit bot Revert of Load data URI images in an async way according to spec (pat…
…chset #2 id:20001 of https://codereview.chromium.org/1492303003/ )

Reason for revert:
Perf regressions: https://code.google.com/p/chromium/issues/detail?id=569023

Original issue's description:
> Load data URI images in an async way according to spec
>
> According to ResponsiveImagesCG/picture-element#223 (comment)
> we seem to be the only browser that loads data URIs immediately (as if they were cached).
> It was introduced as part of the move to async image loading: https://codereview.chromium.org/200923002
>
> This is now causing issues: https://code.google.com/p/chromium/issues/detail?id=469131
>
> This CL changes this behavior to be spec compliant and load data URIs asynchronously, like other resources.
>
> This is bringing https://codereview.chromium.org/1256233002/ to the Chromium repo.
>
> BUG=514206
>
> Committed: https://crrev.com/91aa6fb579a1f656ef274ebe73e426db99b5f52f
> Cr-Commit-Position: refs/heads/master@{#363444}

TBR=pdr@chromium.org,mkwst@chromium.org,esprehn@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=514206

Review URL: https://codereview.chromium.org/1525793006

Cr-Commit-Position: refs/heads/master@{#365498}
c060e8f
@dstockwell dstockwell pushed a commit to dstockwell/chromium that referenced this issue Jul 25, 2016
@yoavweiss yoavweiss + Commit bot Load data URI images in an async way according to spec
According to ResponsiveImagesCG/picture-element#223 (comment)
we seem to be the only browser that loads data URIs immediately (as if they were cached).
It was introduced as part of the move to async image loading: https://codereview.chromium.org/200923002

This is now causing issues: https://code.google.com/p/chromium/issues/detail?id=469131

This CL changes this behavior to be spec compliant and load data URIs asynchronously, like other resources.

This is relanding https://codereview.chromium.org/1492303003

BUG=514206

Review-Url: https://codereview.chromium.org/2173003002
Cr-Commit-Position: refs/heads/master@{#407616}
d0c9ac3
@dstockwell dstockwell pushed a commit to dstockwell/chromium that referenced this issue Aug 8, 2016
@yoavweiss yoavweiss + Commit bot Revert of Load data URI images in an async way according to spec (pat…
…chset #5 id:80001 of https://codereview.chromium.org/2173003002/ )

Reason for revert:
Reverting due to https://bugs.chromium.org/p/chromium/issues/detail?id=632277

Seems like the dev tools' screen capture functionality relies on the sync bahavior of data URIs (issue reproduces with the patch, and does not when reverted)

R=pdr, csharrison

Original issue's description:
> Load data URI images in an async way according to spec
>
> According to ResponsiveImagesCG/picture-element#223 (comment)
> we seem to be the only browser that loads data URIs immediately (as if they were cached).
> It was introduced as part of the move to async image loading: https://codereview.chromium.org/200923002
>
> This is now causing issues: https://code.google.com/p/chromium/issues/detail?id=469131
>
> This CL changes this behavior to be spec compliant and load data URIs asynchronously, like other resources.
>
> This is relanding https://codereview.chromium.org/1492303003
>
> BUG=514206
>
> Committed: https://crrev.com/d0c9ac3bf5bb4843c2c189a72e7b6c39e6743831
> Cr-Commit-Position: refs/heads/master@{#407616}

TBR=pdr@chromium.org,csharrison@chromium.org,nyquist@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=514206

Review-Url: https://codereview.chromium.org/2201313002
Cr-Commit-Position: refs/heads/master@{#410391}
11a418f
@devtools-bot devtools-bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this issue Jan 3, 2017
@yoavweiss yoavweiss + Commit bot Load data URI images in an async way according to spec
According to
ResponsiveImagesCG/picture-element#223 (comment)
we seem to be the only browser that loads data URIs immediately (as if
they were cached).
It was introduced as part of the move to async image loading:
https://codereview.chromium.org/200923002

This is now causing issues:
https://code.google.com/p/chromium/issues/detail?id=469131

This CL changes this behavior to be spec compliant and load data URIs
asynchronously, like other resources.

This is relanding https://codereview.chromium.org/1492303003 and
https://codereview.chromium.org/2173003002/

BUG=514206

Review-Url: https://codereview.chromium.org/2592113003
Cr-Original-Commit-Position: refs/heads/master@{#441184}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: b6403174c6111ea0947dc7cf8b1eb8a6f9e8546c
76fa9dd
@MXEBot MXEBot pushed a commit to mirror/chromium that referenced this issue Jan 4, 2017
@yoavweiss yoavweiss + Commit bot Load data URI images in an async way according to spec
According to
ResponsiveImagesCG/picture-element#223 (comment)
we seem to be the only browser that loads data URIs immediately (as if
they were cached).
It was introduced as part of the move to async image loading:
https://codereview.chromium.org/200923002

This is now causing issues:
https://code.google.com/p/chromium/issues/detail?id=469131

This CL changes this behavior to be spec compliant and load data URIs
asynchronously, like other resources.

This is relanding https://codereview.chromium.org/1492303003 and
https://codereview.chromium.org/2173003002/

BUG=514206

Review-Url: https://codereview.chromium.org/2592113003
Cr-Commit-Position: refs/heads/master@{#441184}
b640317
@babot babot pushed a commit to binaryage/dirac that referenced this issue Jan 4, 2017
@yoavweiss yoavweiss + Commit bot Load data URI images in an async way according to spec
According to
ResponsiveImagesCG/picture-element#223 (comment)
we seem to be the only browser that loads data URIs immediately (as if
they were cached).
It was introduced as part of the move to async image loading:
https://codereview.chromium.org/200923002

This is now causing issues:
https://code.google.com/p/chromium/issues/detail?id=469131

This CL changes this behavior to be spec compliant and load data URIs
asynchronously, like other resources.

This is relanding https://codereview.chromium.org/1492303003 and
https://codereview.chromium.org/2173003002/

BUG=514206

Review-Url: https://codereview.chromium.org/2592113003
Cr-Commit-Position: refs/heads/master@{#441184}
9a88636
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment