Bug 260432 – [MQ5] `Scripting` media initial-only value should never match

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Luke Warlow]

WebKit has implemented the scripting media query but it currently matches `initial-only` when print media type. However, this differs from Firefox, and per https://github.com/w3c/csswg-drafts/issues/8621 it appears like Firefox is correct in never matching this value.

[Tim Nguyen (:ntim)]

It would just be a matter of removing those lines: https://searchfox.org/wubkat/rev/9b7687d4dffc9ecd23f32b4f7c8199f43929d508/Source/WebCore/css/query/MediaQueryFeatures.cpp#610-613

I guess it would make sense if you want the printed content to match the webpage.

[sideshowbarker]

I’m trying to figure out if/how the existing https://wpt.live/css/mediaqueries/scripting.html needs to be updated to test this.

(In reply to Tim Nguyen (:ntim) from comment #1)
> It would just be a matter of removing those lines:
> https://searchfox.org/wubkat/rev/9b7687d4dffc9ecd23f32b4f7c8199f43929d508/
> Source/WebCore/css/query/MediaQueryFeatures.cpp#610-613

If I make that change locally and run https://wpt.live/css/mediaqueries/scripting.html with my build, the “Check that scripting currently matches 'enabled'” test fails.

But when I test with Firefox, it doesn’t fail.

So if Firefox is in fact never matching, then it’s doing that without failing that test case.

And so I’m trying to figure out how WebKit can also be made to not match, but without failing that “Check that scripting currently matches 'enabled'” test case. 

The https://searchfox.org/wubkat/rev/9b7687d4dffc9ecd23f32b4f7c8199f43929d508/Source/WebCore/css/query/MediaQueryFeatures.cpp#610-613 change on its own doesn’t seem sufficient for achieving that.

[Tim Nguyen (:ntim)]

(In reply to Michael[tm] Smith from comment #2)
> I’m trying to figure out if/how the existing
> https://wpt.live/css/mediaqueries/scripting.html needs to be updated to test
> this.
> 
> (In reply to Tim Nguyen (:ntim) from comment #1)
> > It would just be a matter of removing those lines:
> > https://searchfox.org/wubkat/rev/9b7687d4dffc9ecd23f32b4f7c8199f43929d508/
> > Source/WebCore/css/query/MediaQueryFeatures.cpp#610-613
> 
> If I make that change locally and run
> https://wpt.live/css/mediaqueries/scripting.html with my build, the “Check
> that scripting currently matches 'enabled'” test fails.
> 
> But when I test with Firefox, it doesn’t fail.
> 
> So if Firefox is in fact never matching, then it’s doing that without
> failing that test case.
> 
> And so I’m trying to figure out how WebKit can also be made to not match,
> but without failing that “Check that scripting currently matches 'enabled'”
> test case. 
> 
> The
> https://searchfox.org/wubkat/rev/9b7687d4dffc9ecd23f32b4f7c8199f43929d508/
> Source/WebCore/css/query/MediaQueryFeatures.cpp#610-613 change on its own
> doesn’t seem sufficient for achieving that.

I'm surprised that is the case, does your function look like this?

```
            auto& frame = *context.document.frame();

            if (!frame.script().canExecuteScripts(ReasonForCallingCanExecuteScripts::NotAboutToExecuteScript))
                return MatchingIdentifiers { CSSValueNone };

            return MatchingIdentifiers { CSSValueEnabled };
```

[Luke Warlow]

Fwiw I've submitted a patch for this to Chromium which is identical to WebKit's code except without the initial-only handling and it seems be passing all the WPT tests just fine.

[sideshowbarker]

(In reply to Tim Nguyen (:ntim) from comment #3)
> (In reply to Michael[tm] Smith from comment #2)
> > If I make that change locally and run
> > https://wpt.live/css/mediaqueries/scripting.html with my build, the “Check
> > that scripting currently matches 'enabled'” test fails.
>
> I'm surprised that is the case, does your function look like this?

Yeah — and I subsequently cleaned and rebuilt after removing that, and my build’s no longer failing that test case.

But that current test is just checking that it parses. To be able to tell if we ever regress this, it seems like we need an additional test case to check that it doesn’t match. So I’ll try now to put together a test for that.

[sideshowbarker]

Pull request: https://github.com/WebKit/WebKit/pull/16914

[EWS]

Committed 267198@main (706e89411954): <https://commits.webkit.org/267198@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/114340361>