WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
192950
Implement imagesrcset and imagesizes attributes on link rel=preload
https://bugs.webkit.org/show_bug.cgi?id=192950
Summary
Implement imagesrcset and imagesizes attributes on link rel=preload
Dominic Farolino
Reported
2018-12-20 12:46:57 PST
It is useful for the UA to be able to preload responsive images. The idea is to allow this with the imagesrcset and imagesizes attributes on <link rel=preload as=image>. Spec PRs and discussions: -
https://github.com/w3c/preload/issues/120
(Initial discussion) -
https://github.com/whatwg/html/pull/4048
(HTML Standard PR) -
https://github.com/w3c/preload/pull/134
(Preload Spec PR (minor))
Attachments
Patch
(190.96 KB, patch)
2019-03-27 08:38 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2
(2.59 MB, application/zip)
2019-03-27 10:47 PDT
,
EWS Watchlist
no flags
Details
Patch
(192.19 KB, patch)
2019-03-29 09:43 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-highsierra
(2.44 MB, application/zip)
2019-03-29 10:55 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2
(2.73 MB, application/zip)
2019-03-29 11:10 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews112 for mac-highsierra
(2.26 MB, application/zip)
2019-03-29 11:38 PDT
,
EWS Watchlist
no flags
Details
Patch
(192.19 KB, patch)
2019-03-29 15:04 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(150.63 KB, patch)
2019-04-18 03:38 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2
(2.64 MB, application/zip)
2019-04-18 05:42 PDT
,
EWS Watchlist
no flags
Details
Patch
(152.81 KB, patch)
2019-05-14 12:29 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2
(8.08 MB, application/zip)
2019-05-14 14:37 PDT
,
EWS Watchlist
no flags
Details
Patch
(173.62 KB, patch)
2019-05-15 05:46 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2
(2.43 MB, application/zip)
2019-05-15 07:54 PDT
,
EWS Watchlist
no flags
Details
Patch
(175.15 KB, patch)
2019-05-15 11:50 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews213 for win-future
(13.83 MB, application/zip)
2019-05-15 13:49 PDT
,
EWS Watchlist
no flags
Details
Patch
(173.81 KB, patch)
2019-05-15 23:54 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews210 for win-future
(13.35 MB, application/zip)
2019-05-16 01:53 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews126 for ios-simulator-wk2
(2.58 MB, application/zip)
2019-05-16 02:00 PDT
,
EWS Watchlist
no flags
Details
Patch
(175.34 KB, patch)
2019-05-16 02:50 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(177.05 KB, patch)
2019-05-16 12:10 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2
(8.22 MB, application/zip)
2019-05-16 14:13 PDT
,
EWS Watchlist
no flags
Details
Patch
(177.89 KB, patch)
2019-05-17 01:09 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(173.01 KB, patch)
2019-05-17 08:47 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-highsierra
(3.08 MB, application/zip)
2019-05-17 10:01 PDT
,
EWS Watchlist
no flags
Details
Patch
(171.08 KB, patch)
2019-05-17 10:04 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(173.81 KB, patch)
2019-05-17 11:12 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(171.88 KB, patch)
2019-05-17 11:33 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(180.99 KB, patch)
2019-05-31 01:51 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(177.15 KB, patch)
2019-05-31 08:34 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(174.97 KB, patch)
2019-05-31 10:59 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(177.17 KB, patch)
2019-05-31 12:09 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(176.83 KB, patch)
2019-06-02 05:54 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews102 for mac-highsierra
(3.07 MB, application/zip)
2019-06-02 07:10 PDT
,
EWS Watchlist
no flags
Details
Patch
(176.83 KB, patch)
2019-06-02 07:44 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews104 for mac-highsierra-wk2
(2.79 MB, application/zip)
2019-06-02 09:08 PDT
,
EWS Watchlist
no flags
Details
Patch
(176.83 KB, patch)
2019-06-02 09:53 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews101 for mac-highsierra
(3.09 MB, application/zip)
2019-06-02 11:09 PDT
,
EWS Watchlist
no flags
Details
Patch
(176.83 KB, patch)
2019-06-02 12:20 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Show Obsolete
(26)
View All
Add attachment
proposed patch, testcase, etc.
Rob Buis
Comment 1
2019-03-27 08:38:59 PDT
Created
attachment 366072
[details]
Patch
EWS Watchlist
Comment 2
2019-03-27 10:47:43 PDT
Comment on
attachment 366072
[details]
Patch
Attachment 366072
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/11684853
New failing tests: imported/w3c/web-platform-tests/preload/link-header-preload-srcset.tentative.html
EWS Watchlist
Comment 3
2019-03-27 10:47:45 PDT
Created
attachment 366082
[details]
Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Rob Buis
Comment 4
2019-03-29 09:43:30 PDT
Created
attachment 366276
[details]
Patch
EWS Watchlist
Comment 5
2019-03-29 10:55:09 PDT
Comment on
attachment 366276
[details]
Patch
Attachment 366276
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11703692
New failing tests: imported/w3c/web-platform-tests/preload/link-header-preload-srcset.tentative.html
EWS Watchlist
Comment 6
2019-03-29 10:55:11 PDT
Created
attachment 366282
[details]
Archive of layout-test-results from ews100 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-highsierra Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 7
2019-03-29 11:10:18 PDT
Comment on
attachment 366276
[details]
Patch
Attachment 366276
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/11703728
New failing tests: imported/w3c/web-platform-tests/preload/link-header-preload-srcset.tentative.html
EWS Watchlist
Comment 8
2019-03-29 11:10:20 PDT
Created
attachment 366283
[details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 9
2019-03-29 11:38:48 PDT
Comment on
attachment 366276
[details]
Patch
Attachment 366276
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11703770
New failing tests: imported/w3c/web-platform-tests/preload/link-header-preload-srcset.tentative.html
EWS Watchlist
Comment 10
2019-03-29 11:38:50 PDT
Created
attachment 366285
[details]
Archive of layout-test-results from ews112 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-highsierra Platform: Mac OS X 10.13.6
Rob Buis
Comment 11
2019-03-29 15:04:31 PDT
Created
attachment 366308
[details]
Patch
Rob Buis
Comment 12
2019-04-02 07:52:45 PDT
(In reply to Rob Buis from
comment #11
)
> Created
attachment 366308
[details]
> Patch
Note that the parameter list for loadLink is quite long, it may make sense to consolidate them in a single class, but I think it should be a separate patch.
Rob Buis
Comment 13
2019-04-18 03:38:57 PDT
Created
attachment 367724
[details]
Patch
EWS Watchlist
Comment 14
2019-04-18 05:42:28 PDT
Comment on
attachment 367724
[details]
Patch
Attachment 367724
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/11913509
New failing tests: imported/w3c/web-platform-tests/preload/link-header-preload-srcset.tentative.html
EWS Watchlist
Comment 15
2019-04-18 05:42:30 PDT
Created
attachment 367725
[details]
Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Rob Buis
Comment 16
2019-05-09 08:26:43 PDT
Would it be better to have a runtime flag for this patch?
Rob Buis
Comment 17
2019-05-13 10:46:10 PDT
Note that the ios-sim failure is likely because of different viewport sizes compared to the other platforms. I'll see if I can change the test or add correct expectation tomorrow.
youenn fablet
Comment 18
2019-05-13 13:55:32 PDT
Comment on
attachment 367724
[details]
Patch Patch looks good to me overall. Has this shipped in other browsers? View in context:
https://bugs.webkit.org/attachment.cgi?id=367724&action=review
> Source/WebCore/loader/LinkHeader.cpp:290 > case LinkParameterMedia:
LinkHeader::setValue should probably be String&&
> Source/WebCore/loader/LinkHeader.cpp:297 > + m_imageSizes = value;
move value here and above
> Source/WebCore/loader/LinkLoader.h:53 > + bool loadLink(const LinkRelAttribute&, const URL&, const String& as, const String& media, const String& type, const String& crossOrigin, const String&, const String&, Document&);
Maybe give the names here for readability.
> LayoutTests/imported/w3c/web-platform-tests/preload/link-header-preload-nonce.html:15 > + }, 5000);
This test will take 5 seconds to load. Some will take 3 seconds or 5 seconds. Is there a way we can run the test faster? Like have a Link preload in the HTML and do these validation checks once its onload event handler is called. Or loop until all validation checks pass (and exit after a given number of iterations instead of timing out).
> LayoutTests/imported/w3c/web-platform-tests/preload/link-header-preload-srcset.tentative-expected.txt:3 > +CONSOLE MESSAGE: The resource
http://localhost:8800/preload/resources/square.png?from-header&300
was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it wasn't preloaded for nothing.
Is there a possibility such messages will make the tests flaky? Should we use DumpJSConsoleLogInStdErr? Ditto for other tests.
Rob Buis
Comment 19
2019-05-14 12:29:42 PDT
Created
attachment 369880
[details]
Patch
Rob Buis
Comment 20
2019-05-14 13:34:07 PDT
Comment on
attachment 367724
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=367724&action=review
>> Source/WebCore/loader/LinkHeader.cpp:290 >> case LinkParameterMedia: > > LinkHeader::setValue should probably be String&&
Done.
>> Source/WebCore/loader/LinkHeader.cpp:297 >> + m_imageSizes = value; > > move value here and above
Done.
>> Source/WebCore/loader/LinkLoader.h:53 >> + bool loadLink(const LinkRelAttribute&, const URL&, const String& as, const String& media, const String& type, const String& crossOrigin, const String&, const String&, Document&); > > Maybe give the names here for readability.
Done.
>> LayoutTests/imported/w3c/web-platform-tests/preload/link-header-preload-nonce.html:15 >> + }, 5000); > > This test will take 5 seconds to load. Some will take 3 seconds or 5 seconds. > Is there a way we can run the test faster? > Like have a Link preload in the HTML and do these validation checks once its onload event handler is called. > Or loop until all validation checks pass (and exit after a given number of iterations instead of timing out).
Isn't it the case that a Link preload in the HTML would not be guaranteed to finish after the Link headers? For now I am experimenting with iterations as you can see in the updated link-header-preload-nonce.html. However note that in the FAIL case this could still be a similar delay equal to the time out, i.e I do 10 loops of 500ms timeouts so in FAIL case it still takes 5s. link-header-preload-nonce.html is such a FAIL, I have not looked into why, but I think this patch is not the right one since it may be csp related.
>> LayoutTests/imported/w3c/web-platform-tests/preload/link-header-preload-srcset.tentative-expected.txt:3 >> +CONSOLE MESSAGE: The resource
http://localhost:8800/preload/resources/square.png?from-header&300
was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it wasn't preloaded for nothing. > > Is there a possibility such messages will make the tests flaky? > Should we use DumpJSConsoleLogInStdErr? > Ditto for other tests.
It is possible with iterations/shorted timeout these messages will go away. FWIW I did not see DumpJSConsoleLogInStdErr being used in wpt so far.
Rob Buis
Comment 21
2019-05-14 13:39:36 PDT
(In reply to youenn fablet from
comment #18
)
> Comment on
attachment 367724
[details]
> Patch > > Has this shipped in other browsers?
It has shipped in Chrome:
https://chromestatus.com/features/5164259990306816
It seems Firefox has no objections but needs to implement rel=preload first:
https://github.com/mozilla/standards-positions/issues/130
https://bugzilla.mozilla.org/show_bug.cgi?id=1515760
EWS Watchlist
Comment 22
2019-05-14 14:37:47 PDT
Comment on
attachment 369880
[details]
Patch
Attachment 369880
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/12190737
New failing tests: imported/w3c/web-platform-tests/preload/link-header-preload-srcset.tentative.html
EWS Watchlist
Comment 23
2019-05-14 14:37:49 PDT
Created
attachment 369896
[details]
Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.4
Frédéric Wang (:fredw)
Comment 24
2019-05-15 00:18:50 PDT
Comment on
attachment 369880
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=369880&action=review
> Source/WebCore/html/HTMLLinkElement.cpp:265 > + if (!m_linkLoader.loadLink(m_relAttribute, url, attributeWithoutSynchronization(asAttr), attributeWithoutSynchronization(mediaAttr), attributeWithoutSynchronization(typeAttr), attributeWithoutSynchronization(crossoriginAttr), attributeWithoutSynchronization(imagesrcsetAttr), attributeWithoutSynchronization(imagesizesAttr), document()))
The loadLink lines are becoming super long. I thought there was a coding style guideline to avoid that, but I only found
https://webkit.org/code-style-guidelines/#indentation-wrap-bool-op
Rob Buis
Comment 25
2019-05-15 05:46:01 PDT
Created
attachment 369947
[details]
Patch
EWS Watchlist
Comment 26
2019-05-15 07:53:59 PDT
Comment on
attachment 369947
[details]
Patch
Attachment 369947
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/12197067
New failing tests: imported/w3c/web-platform-tests/preload/link-header-preload-imagesrcset.html
EWS Watchlist
Comment 27
2019-05-15 07:54:01 PDT
Created
attachment 369952
[details]
Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.4
Rob Buis
Comment 28
2019-05-15 11:50:22 PDT
Created
attachment 369976
[details]
Patch
EWS Watchlist
Comment 29
2019-05-15 13:49:09 PDT
Comment on
attachment 369976
[details]
Patch
Attachment 369976
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/12199967
New failing tests: js/dom/custom-constructors.html
EWS Watchlist
Comment 30
2019-05-15 13:49:15 PDT
Created
attachment 369989
[details]
Archive of layout-test-results from ews213 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews213 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
youenn fablet
Comment 31
2019-05-15 14:12:07 PDT
Comment on
attachment 369976
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=369976&action=review
> Source/WebCore/loader/LinkLoader.cpp:250 > + url = href;
s/url = href/url = document.completeURL(href)/
> Source/WebCore/loader/LinkLoader.cpp:253 > + document.addConsoleMessage(MessageSource::Other, MessageLevel::Error, "<link rel=preload> has an invalid `href` value"_s);
This is a bit misleading since we are talking in the message of href, while it might be related to imageSrcSet value. Can we improve on this, either by adding the URL that is invalid or by making the message more specific.
> Source/WebCore/loader/LinkLoader.cpp:262 > + auto linkRequest = createPotentialAccessControlRequest(document.completeURL(url), document, crossOriginMode, WTFMove(options));
s/document.completeURL(url)/url
> LayoutTests/ChangeLog:11 > + * platform/ios-simulator-12-wk2/imported/w3c/web-platform-tests/preload/link-header-preload-imagesrcset-expected.txt: Added.
It should probably be ios-simulator-wk2 instead of ios-simulator-12-wk2. Is there a way to modify the test to handle various dimensions?
Rob Buis
Comment 32
2019-05-15 23:54:12 PDT
Created
attachment 370026
[details]
Patch
EWS Watchlist
Comment 33
2019-05-16 01:53:14 PDT
Comment on
attachment 370026
[details]
Patch
Attachment 370026
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/12205022
New failing tests: js/dom/custom-constructors.html
EWS Watchlist
Comment 34
2019-05-16 01:53:16 PDT
Created
attachment 370030
[details]
Archive of layout-test-results from ews210 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews210 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
EWS Watchlist
Comment 35
2019-05-16 02:00:51 PDT
Comment on
attachment 370026
[details]
Patch
Attachment 370026
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/12204984
New failing tests: imported/w3c/web-platform-tests/preload/link-header-preload-imagesrcset.html
EWS Watchlist
Comment 36
2019-05-16 02:00:53 PDT
Created
attachment 370031
[details]
Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.4
Rob Buis
Comment 37
2019-05-16 02:50:24 PDT
Created
attachment 370034
[details]
Patch
Rob Buis
Comment 38
2019-05-16 12:10:38 PDT
Created
attachment 370053
[details]
Patch
EWS Watchlist
Comment 39
2019-05-16 14:13:28 PDT
Comment on
attachment 370053
[details]
Patch
Attachment 370053
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/12209105
New failing tests: imported/w3c/web-platform-tests/preload/dynamic-adding-preload-imagesrcset.html
EWS Watchlist
Comment 40
2019-05-16 14:13:30 PDT
Created
attachment 370068
[details]
Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.5
Rob Buis
Comment 41
2019-05-17 01:09:51 PDT
Created
attachment 370109
[details]
Patch
Rob Buis
Comment 42
2019-05-17 08:47:18 PDT
Created
attachment 370119
[details]
Patch
EWS Watchlist
Comment 43
2019-05-17 10:01:04 PDT
Comment on
attachment 370119
[details]
Patch
Attachment 370119
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/12215917
New failing tests: imported/w3c/web-platform-tests/preload/single-download-preload.html
EWS Watchlist
Comment 44
2019-05-17 10:01:06 PDT
Created
attachment 370123
[details]
Archive of layout-test-results from ews100 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-highsierra Platform: Mac OS X 10.13.6
Rob Buis
Comment 45
2019-05-17 10:04:30 PDT
Created
attachment 370124
[details]
Patch
Rob Buis
Comment 46
2019-05-17 11:12:59 PDT
Created
attachment 370127
[details]
Patch
Rob Buis
Comment 47
2019-05-17 11:33:19 PDT
Created
attachment 370130
[details]
Patch
Rob Buis
Comment 48
2019-05-17 13:05:35 PDT
Comment on
attachment 369976
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=369976&action=review
>> Source/WebCore/loader/LinkLoader.cpp:250 >> + url = href; > > s/url = href/url = document.completeURL(href)/
Done.
>> Source/WebCore/loader/LinkLoader.cpp:253 >> + document.addConsoleMessage(MessageSource::Other, MessageLevel::Error, "<link rel=preload> has an invalid `href` value"_s); > > This is a bit misleading since we are talking in the message of href, while it might be related to imageSrcSet value. > Can we improve on this, either by adding the URL that is invalid or by making the message more specific.
Done.
>> Source/WebCore/loader/LinkLoader.cpp:262 >> + auto linkRequest = createPotentialAccessControlRequest(document.completeURL(url), document, crossOriginMode, WTFMove(options)); > > s/document.completeURL(url)/url
Done.
>> LayoutTests/ChangeLog:11 >> + * platform/ios-simulator-12-wk2/imported/w3c/web-platform-tests/preload/link-header-preload-imagesrcset-expected.txt: Added. > > It should probably be ios-simulator-wk2 instead of ios-simulator-12-wk2. > Is there a way to modify the test to handle various dimensions?
Unfortunately I did not see a way to modify the test.
WebKit Commit Bot
Comment 49
2019-05-17 13:34:49 PDT
Comment on
attachment 370130
[details]
Patch Clearing flags on attachment: 370130 Committed
r245475
: <
https://trac.webkit.org/changeset/245475
>
WebKit Commit Bot
Comment 50
2019-05-17 13:34:51 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 51
2019-05-17 13:40:14 PDT
<
rdar://problem/50904042
>
Truitt Savell
Comment 52
2019-05-21 09:58:08 PDT
The newly imported imported/w3c/web-platform-tests/preload/link-header-preload-delay-onload.html from
https://trac.webkit.org/changeset/245475/webkit
is flakey History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fpreload%2Flink-header-preload-delay-onload.html
Diff: --- /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/preload/link-header-preload-delay-onload-expected.txt +++ /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/preload/link-header-preload-delay-onload-actual.txt @@ -1,4 +1,4 @@ -PASS Makes sure that Link headers preload resources and block window.onload after resource discovery +FAIL Makes sure that Link headers preload resources and block window.onload after resource discovery assert_true: expected true got false
Rob Buis
Comment 53
2019-05-21 10:15:49 PDT
(In reply to Truitt Savell from
comment #52
)
> -PASS Makes sure that Link headers preload resources and block window.onload > after resource discovery > +FAIL Makes sure that Link headers preload resources and block window.onload > after resource discovery assert_true: expected true got false
It is probably best to skip this test and make a bug to fix the test result. I should have some time to look into it end of the week.
Jon Lee
Comment 54
2019-05-28 13:52:24 PDT
(In reply to Rob Buis from
comment #53
)
> (In reply to Truitt Savell from
comment #52
) > > -PASS Makes sure that Link headers preload resources and block window.onload > > after resource discovery > > +FAIL Makes sure that Link headers preload resources and block window.onload > > after resource discovery assert_true: expected true got false > > It is probably best to skip this test and make a bug to fix the test result. > I should have some time to look into it end of the week.
Can we wrap this in an experimental feature flag?
Alexey Proskuryakov
Comment 55
2019-05-28 14:01:12 PDT
Let's roll back at this point. The test is still flaky, and yes, features need flags.
Shawn Roberts
Comment 56
2019-05-28 14:10:53 PDT
Reverted
r245475
for reason: Newly imported test is flaky. Features need flags. Committed
r245825
: <
https://trac.webkit.org/changeset/245825
>
Rob Buis
Comment 57
2019-05-31 01:51:21 PDT
Created
attachment 371044
[details]
Patch
Rob Buis
Comment 58
2019-05-31 08:34:21 PDT
Created
attachment 371059
[details]
Patch
Rob Buis
Comment 59
2019-05-31 10:59:43 PDT
Created
attachment 371070
[details]
Patch
Rob Buis
Comment 60
2019-05-31 12:09:55 PDT
Created
attachment 371073
[details]
Patch
Rob Buis
Comment 61
2019-05-31 13:58:34 PDT
Comment on
attachment 371073
[details]
Patch Sorry for not including the runtime flag in the patch that was reverted! I added it in the latest patch. I also removed the flaky test, since it is not actually testing imagesrcset/imagesizes.
youenn fablet
Comment 62
2019-05-31 15:34:52 PDT
Comment on
attachment 371073
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=371073&action=review
> Source/WebCore/loader/LinkLoader.cpp:245 > + if (type == CachedResource::Type::ImageResource && !imageSrcSet.isEmpty()) {
We probably need to add a runtime check here to disable the behavior if the flag is not enabled.
> Source/WebCore/loader/LinkLoader.h:66 > + static std::unique_ptr<LinkPreloadResourceClient> preloadIfNeeded(const LinkRelAttribute&, const URL& href, Document&, const String& as, const String& media, const String& type, const String& crossOriginMode, const String&, const String&, LinkLoader*);
Could we add the names of the string variables to improve readability.
> LayoutTests/imported/w3c/ChangeLog:66 > +
Not needed.
> LayoutTests/platform/ios-simulator-12-wk2/imported/w3c/web-platform-tests/preload/dynamic-adding-preload-imagesrcset-expected.txt:2 > +FAIL Makes sure that a dynamically added preload with imagesrcset works assert_equals: resources/square.png?400 expected 1 but got 0
Could we add specific variations of these tests for iOS?
Rob Buis
Comment 63
2019-06-02 05:54:34 PDT
Created
attachment 371145
[details]
Patch
EWS Watchlist
Comment 64
2019-06-02 07:10:34 PDT
Comment on
attachment 371145
[details]
Patch
Attachment 371145
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/12355653
New failing tests: inspector/canvas/recording.html
EWS Watchlist
Comment 65
2019-06-02 07:10:37 PDT
Created
attachment 371148
[details]
Archive of layout-test-results from ews102 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6
Rob Buis
Comment 66
2019-06-02 07:44:43 PDT
Created
attachment 371149
[details]
Patch
EWS Watchlist
Comment 67
2019-06-02 09:08:12 PDT
Comment on
attachment 371149
[details]
Patch
Attachment 371149
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/12356219
New failing tests: http/wpt/service-workers/service-worker-networkprocess-crash.html
EWS Watchlist
Comment 68
2019-06-02 09:08:15 PDT
Created
attachment 371156
[details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Rob Buis
Comment 69
2019-06-02 09:53:13 PDT
Created
attachment 371157
[details]
Patch
EWS Watchlist
Comment 70
2019-06-02 11:09:40 PDT
Comment on
attachment 371157
[details]
Patch
Attachment 371157
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/12356774
New failing tests: inspector/canvas/recording.html
EWS Watchlist
Comment 71
2019-06-02 11:09:43 PDT
Created
attachment 371159
[details]
Archive of layout-test-results from ews101 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
Rob Buis
Comment 72
2019-06-02 12:18:38 PDT
Comment on
attachment 371073
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=371073&action=review
>> Source/WebCore/loader/LinkLoader.cpp:245 >> + if (type == CachedResource::Type::ImageResource && !imageSrcSet.isEmpty()) { > > We probably need to add a runtime check here to disable the behavior if the flag is not enabled.
Done.
>> Source/WebCore/loader/LinkLoader.h:66 >> + static std::unique_ptr<LinkPreloadResourceClient> preloadIfNeeded(const LinkRelAttribute&, const URL& href, Document&, const String& as, const String& media, const String& type, const String& crossOriginMode, const String&, const String&, LinkLoader*); > > Could we add the names of the string variables to improve readability.
Done.
>> LayoutTests/imported/w3c/ChangeLog:66 >> + > > Not needed.
Removed.
>> LayoutTests/platform/ios-simulator-12-wk2/imported/w3c/web-platform-tests/preload/dynamic-adding-preload-imagesrcset-expected.txt:2 >> +FAIL Makes sure that a dynamically added preload with imagesrcset works assert_equals: resources/square.png?400 expected 1 but got 0 > > Could we add specific variations of these tests for iOS?
I still don't know how to do that. If anybody has an idea, I can do a follow-up patch.
Rob Buis
Comment 73
2019-06-02 12:20:01 PDT
Created
attachment 371161
[details]
Patch
WebKit Commit Bot
Comment 74
2019-06-03 13:50:00 PDT
Comment on
attachment 371161
[details]
Patch Clearing flags on attachment: 371161 Committed
r246045
: <
https://trac.webkit.org/changeset/246045
>
WebKit Commit Bot
Comment 75
2019-06-03 13:50:03 PDT
All reviewed patches have been landed. Closing bug.
youenn fablet
Comment 76
2019-06-03 13:50:20 PDT
> >> LayoutTests/platform/ios-simulator-12-wk2/imported/w3c/web-platform-tests/preload/dynamic-adding-preload-imagesrcset-expected.txt:2 > >> +FAIL Makes sure that a dynamically added preload with imagesrcset works assert_equals: resources/square.png?400 expected 1 but got 0 > > > > Could we add specific variations of these tests for iOS? > > I still don't know how to do that. If anybody has an idea, I can do a > follow-up patch.
IIUC the issue, the viewport for iOS simulator is different, which will trigger preloading of different resources than the test expects. I would copy the test in some LayoutTests iOS specific folder and update the test so that we validate preloading with the iOS simulator viewport works. Could the test get the viewport and from it, compute on its own which preload should be selected? Alternatively, the test could be copie-then-made-iOS-specific.
Rob Buis
Comment 77
2019-06-03 13:53:21 PDT
(In reply to youenn fablet from
comment #76
)
> > >> LayoutTests/platform/ios-simulator-12-wk2/imported/w3c/web-platform-tests/preload/dynamic-adding-preload-imagesrcset-expected.txt:2 > > >> +FAIL Makes sure that a dynamically added preload with imagesrcset works assert_equals: resources/square.png?400 expected 1 but got 0 > > > > > > Could we add specific variations of these tests for iOS? > > > > I still don't know how to do that. If anybody has an idea, I can do a > > follow-up patch. > > IIUC the issue, the viewport for iOS simulator is different, which will > trigger preloading of different resources than the test expects. > I would copy the test in some LayoutTests iOS specific folder and update the > test so that we validate preloading with the iOS simulator viewport works. > > Could the test get the viewport and from it, compute on its own which > preload should be selected? > > Alternatively, the test could be copie-then-made-iOS-specific.
Ok, I will have a look tomorrow.
Rob Buis
Comment 78
2019-06-05 02:26:23 PDT
(In reply to Rob Buis from
comment #77
)
> (In reply to youenn fablet from
comment #76
) > > IIUC the issue, the viewport for iOS simulator is different, which will > > trigger preloading of different resources than the test expects. > > I would copy the test in some LayoutTests iOS specific folder and update the > > test so that we validate preloading with the iOS simulator viewport works. > > > > Could the test get the viewport and from it, compute on its own which > > preload should be selected? > > > > Alternatively, the test could be copie-then-made-iOS-specific. > > Ok, I will have a look tomorrow.
I fixed the tests in the WPT repo and imported them here:
https://bugs.webkit.org/show_bug.cgi?id=198533
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug