Skip to content

<link> elements with media queries that do not affect the current page can be delayed #9746

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

Conversation

anttijk
Copy link
Contributor

@anttijk anttijk commented Feb 7, 2023

9e31d4e

<link> elements with media queries that do not affect the current page can be delayed
https://bugs.webkit.org/show_bug.cgi?id=39455
rdar://102178226

Reviewed by Alan Baradlay.

We correctly deprioritize stylesheets with non-matching media attribute and don't make them
rendering blocking. However they may still delay the visually non-empty milestone. This delay
only comes into play if there is not enough visual content to trigger the milestone otherwise.
This may end up delaying rendering until the non-matching stylesheets are fully loaded on simple
pages.

* LayoutTests/http/tests/css/link-with-non-matching-media-slow-load-expected.html: Added.
* LayoutTests/http/tests/css/link-with-non-matching-media-slow-load.html: Added.
* Source/WebCore/page/FrameView.cpp:
(WebCore::FrameView::checkAndDispatchDidReachVisuallyNonEmptyState):

Ignore "very low" priority resources when determining if everything important is loaded.

* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::isVisuallyNonEmpty const):
* Source/WebCore/testing/Internals.h:
* Source/WebCore/testing/Internals.idl:

Testing support.

Canonical link: https://commits.webkit.org/259963@main

f30ff97

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

@anttijk anttijk requested a review from cdumez as a code owner February 7, 2023 11:42
@anttijk anttijk self-assigned this Feb 7, 2023
@anttijk anttijk added the Page Loading For bugs in page loading, including handling of network callbacks. label Feb 7, 2023
@pepelsbey
Copy link

Nice! Does it affect CSS files loaded via @import? There’s a similar issue in Chromium. These two should work the same:

<link rel="stylesheet" href="/style.css" media="(prefers-color-scheme: light)">

<style>
  @import url(import.css) (prefers-color-scheme: light)
</style>

@anttijk
Copy link
Contributor Author

anttijk commented Feb 7, 2023

Nice! Does it affect CSS files loaded via @import? There’s a similar issue in Chromium. These two should work the same:

No, this PR is only about <link>. Currently media queries do not affect loading of @import rules at all, just applying. Worth a separate bug.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 7, 2023
@anttijk anttijk added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Feb 7, 2023
…e can be delayed

https://bugs.webkit.org/show_bug.cgi?id=39455
rdar://102178226

Reviewed by Alan Baradlay.

We correctly deprioritize stylesheets with non-matching media attribute and don't make them
rendering blocking. However they may still delay the visually non-empty milestone. This delay
only comes into play if there is not enough visual content to trigger the milestone otherwise.
This may end up delaying rendering until the non-matching stylesheets are fully loaded on simple
pages.

* LayoutTests/http/tests/css/link-with-non-matching-media-slow-load-expected.html: Added.
* LayoutTests/http/tests/css/link-with-non-matching-media-slow-load.html: Added.
* Source/WebCore/page/FrameView.cpp:
(WebCore::FrameView::checkAndDispatchDidReachVisuallyNonEmptyState):

Ignore "very low" priority resources when determining if everything important is loaded.

* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::isVisuallyNonEmpty const):
* Source/WebCore/testing/Internals.h:
* Source/WebCore/testing/Internals.idl:

Testing support.

Canonical link: https://commits.webkit.org/259963@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the visually-non-empty-stylesheet-media branch from f30ff97 to 9e31d4e Compare February 7, 2023 17:33
@webkit-commit-queue
Copy link
Collaborator

Committed 259963@main (9e31d4e): https://commits.webkit.org/259963@main

Reviewed commits have been landed. Closing PR #9746 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 9e31d4e into WebKit:main Feb 7, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 7, 2023
@anttijk anttijk deleted the visually-non-empty-stylesheet-media branch February 9, 2023 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Page Loading For bugs in page loading, including handling of network callbacks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants