Re: Web Platform Test unexpectedly passing in Firefox

105 views
Skip to first unread message

Emilio Cobos Álvarez

unread,
Feb 17, 2023, 4:57:33 AM2/17/23
to Yash Joshi, dev-pl...@mozilla.org, yoav...@chromium.org
Hi Yash, thanks for reaching out.

Moving the thread to dev-platform@ which is the right mailing-list for Gecko stuff, and cc'ing Yoav who reviewed the chromium patch.

So, what might happen (and pretty sure happens) is that we really start fetching the @import rule earlier than the first script runs, due to the import scanner that runs during parsing, introduced in bug 1546783.

We ignore the media list in this scanner, because @import url() media really is blocking the rendering regardless of media, so we need to load it asap anyways. There's a test here for that.

That explains that the test passes in some cases, depending on timing.

That said, I'm not convinced the test is correct. The spec doesn't mention @import's media list at all, and more importantly, @import can be mixed with other rules (or other imports) not covered by its media query, and the CSSOM doesn't have a concept for having a partially-loaded stylesheet. So if you have:

  @import url("print.css") print;
  div { color: green }

It seems we'd need to block on the import. The right way for authors if they want a single @import that doesn't block rendering would be something like:

<style media="print">


Cheers,

 -- Emilio

On Fri, Feb 17, 2023 at 7:25 AM Yash Joshi <yashjo...@gmail.com> wrote:
Hello Firefox devs,
I am Yash Joshi, an undergrad student from India. I recently wrote my first web platform test which checks if browsers wait for delayed stylesheets with non matching Media Queries. Every Single browser I tested [chromium safari and firefox] has not yet implemented the said feature.
I thus wrote a simple WPT which basically delays serving the stylesheet {with non matching MQ} by 1s and checks if browser continued JS execution in the meantime or waited for the stylesheet to load [test failed in this case] and all browsers fail test [expected] and firefox somehow passes this [which should not be the case].
I am attaching relevant links and detailed context below for your reference. Pls let me know what am I missing here or if there is something wrong with my approach.
Also, pls direct me to correct channels if this is not the right place to discuss this.
Thanks a lot for your consideration.
-------------------------------------------------------------------Details------------------------------------------------------------

Relevant Specification: Assigning property values, Cascading, and Inheritance (w3.org) and  CSS Cascading and Inheritance Level 5 (w3c.github.io)

Live Test Link (wait for 10s to load): and check the console panel for wait time : : https://cheerful-capybara-92469b.netlify.app [Source Code: Github Link for test code]

WPT Status : web-platform-tests dashboard (wpt.fyi)

Github Link:  Add WPT to check that browser lazy loads non-matching Media Query by chromium-wpt-export-bot · Pull Request #38370 · web-platform-tests/wpt (github.com)
Gerrit Link: 
 Add WPT to check that browser lazy loads non-matching Media Query (4224891) ·] Used trickle pipe for delay of 1s.

--
You received this message because you are subscribed to the Google Groups "firef...@mozilla.org" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firefox-dev...@mozilla.org.
To view this discussion on the web visit https://groups.google.com/a/mozilla.org/d/msgid/firefox-dev/9aaf45d3-39c9-4a09-9129-d482c8606664n%40mozilla.org.

Yoav Weiss

unread,
Feb 17, 2023, 5:27:24 AM2/17/23
to Emilio Cobos Álvarez, Yash Joshi, dev-pl...@mozilla.org
(re-replying from an address that's actually on this list)

Thanks Emilio for looping me in. I missed your comment on the Chromium issue this was merged into. Apologies for that!

From a pure performance perspective, it seems like it'd be better for browsers to not block rendering and JS execution on such non-matching imports. But you're right that this would require a CSSOM behavior change that I didn't consider initially. I *think* that change could mean that the CSSImportRule is present, but its `styleSheet` is initially null. 
I'd expect that to be web compatible, but I agree this would require a spec change to allow for that behavior, as well as tests that verify that this is the case.


On Fri, Feb 17, 2023 at 10:59 AM Emilio Cobos Álvarez <emi...@mozilla.com> wrote:
Hi Yash, thanks for reaching out.

Moving the thread to dev-platform@ which is the right mailing-list for Gecko stuff, and cc'ing Yoav who reviewed the chromium patch.

So, what might happen (and pretty sure happens) is that we really start fetching the @import rule earlier than the first script runs, due to the import scanner that runs during parsing, introduced in bug 1546783.

Chromium has a similar scanner and similarly, I don't believe it takes @import media rules into account. So there may be a different explanation for the flakiness.

 
You received this message because you are subscribed to the Google Groups "dev-pl...@mozilla.org" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dev-platform...@mozilla.org.
To view this discussion on the web visit https://groups.google.com/a/mozilla.org/d/msgid/dev-platform/CAFhp-qf5RWjh3TbNZ%3DsTOUk2xwnY2V1DOpLhcAG%3Dwhn0bHSYVg%40mail.gmail.com.

Emilio Cobos Álvarez

unread,
Feb 17, 2023, 7:02:04 AM2/17/23
to Yoav Weiss, Yash Joshi, dev-pl...@mozilla.org
On 2/17/23 11:27, Yoav Weiss wrote:
> (re-replying from an address that's actually on this list)
>
> Thanks Emilio for looping me in. I missed your comment
> <https://bugs.chromium.org/p/chromium/issues/detail?id=1001078#c3> on
> the Chromium issue this was merged into. Apologies for that!
>
> From a pure performance perspective, it seems like it'd be better for
> browsers to not block rendering and JS execution on such non-matching
> imports. But you're right that this would require a CSSOM behavior
> change that I didn't consider initially. I *think* that change could
> mean that the CSSImportRule is present, but its `styleSheet` is
> initially null.
> I'd expect that to be web compatible, but I agree this would require a
> spec change to allow for that behavior, as well as tests that verify
> that this is the case.

I agree in principle it'd be useful, though it's a bit sketchy to
partially apply a stylesheet before its load event has fired, etc.

I'm curious about whether you have data to see if this is a common
pattern (intuitively I'd expect conditional @import on inline style
elements rather uncommon, given there's <link> too)...

Also, presumably we'd want this to work for `<link>` too, right? That
is, <link rel="stylesheet" href="stuff.css">, where `stuff.css` has
something like:

@import url("print.css") print;

Seems like the same issue as inline style to me, and maybe a bit more
common.

Anyways, it seems this discussion should happen somewhere in the HTML or
CSS spec issue trackers :)

-- Emilio

> On Fri, Feb 17, 2023 at 10:59 AM Emilio Cobos Álvarez
> Chromium has a similar scanner and similarly, I don't believe it
> takes @import media rules into account. So there may be a different
> explanation for the flakiness.

Chromium's scanner doesn't load conditional imports[1], we do. That
explains why the Flakiness is Firefox-specific.

-- Emilio

[1]:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/parser/css_preload_scanner.cc;l=290;drc=4ced8912f6c8b32715984dd81e3f376a260ebab6


>
>
> We ignore the media list in this scanner, because @import url()
> media really is blocking the rendering regardless of media, so we
> need to load it asap anyways. There's a test
> <https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/layout/style/test/gtest/ImportScannerTest.cpp#52> here for that.
>
> That explains that the test passes in some cases, depending on timing.
>
> That said, I'm not convinced the test is correct. The spec
> <https://html.spec.whatwg.org/#interactions-of-styling-and-scripting> doesn't mention @import's media list at all, and more importantly, @import can be mixed with other rules (or other imports) not covered by its media query, and the CSSOM doesn't have a concept for having a partially-loaded stylesheet. So if you have:
> <https://www.w3.org/TR/CSS2/cascade.html#at-import>__and CSS
> Cascading and Inheritance Level 5 (w3c.github.io)
> <https://w3c.github.io/csswg-drafts/css-cascade-5/#at-import>
>
> Live Test Link (wait for 10s to load): and check the console
> panel for wait time : :
> https://cheerful-capybara-92469b.netlify.app
> <https://cheerful-capybara-92469b.netlify.app/> [Source Code:
> Github Link for test code
> <https://github.com/yashjoshi-dotcom/WPT-test-for-non-matching-MQ-001>]
>
> WPT Status : web-platform-tests dashboard (wpt.fyi)
> <https://wpt.fyi/results/css/mediaqueries/mq-non-matching-lazy-load.html?label=experimental&label=master&aligned&view=subtest>
>
> Github Link: Add WPT to check that browser lazy loads
> non-matching Media Query by chromium-wpt-export-bot · Pull
> Request #38370 · web-platform-tests/wpt (github.com)
> <https://github.com/web-platform-tests/wpt/pull/38370>
> Gerrit Link: Add WPT to check that browser lazy loads
> non-matching Media Query (4224891) ·
> <https://chromium-review.googlesource.com/c/chromium/src/+/4224891>] Used trickle pipe for delay of 1s.
>
> --
> You received this message because you are subscribed to the
> Google Groups "firef...@mozilla.org
> <mailto:firef...@mozilla.org>" group.
> To unsubscribe from this group and stop receiving emails from
> it, send an email to firefox-dev...@mozilla.org
> <mailto:firefox-dev...@mozilla.org>.
> To view this discussion on the web visit
> https://groups.google.com/a/mozilla.org/d/msgid/firefox-dev/9aaf45d3-39c9-4a09-9129-d482c8606664n%40mozilla.org <https://groups.google.com/a/mozilla.org/d/msgid/firefox-dev/9aaf45d3-39c9-4a09-9129-d482c8606664n%40mozilla.org?utm_medium=email&utm_source=footer>.
>
> --
> You received this message because you are subscribed to the Google
> Groups "dev-pl...@mozilla.org <mailto:dev-pl...@mozilla.org>"
> group.
> To unsubscribe from this group and stop receiving emails from it,
> send an email to dev-platform...@mozilla.org
> <mailto:dev-platform...@mozilla.org>.
> To view this discussion on the web visit
> https://groups.google.com/a/mozilla.org/d/msgid/dev-platform/CAFhp-qf5RWjh3TbNZ%3DsTOUk2xwnY2V1DOpLhcAG%3Dwhn0bHSYVg%40mail.gmail.com <https://groups.google.com/a/mozilla.org/d/msgid/dev-platform/CAFhp-qf5RWjh3TbNZ%3DsTOUk2xwnY2V1DOpLhcAG%3Dwhn0bHSYVg%40mail.gmail.com?utm_medium=email&utm_source=footer>.
>
Reply all
Reply to author
Forward
0 new messages