1460870 - browser_parsable_css.js | Got error message for resource://pdf.js/web/viewer.css when Gecko 62 merges to Beta on 2018-06-14

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Eliza Balazs [:ebalazs_]]

Central as Beta: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bed328ba3d401474c773891f7a76dcae6ac56a41&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable

This is a regression from bug 509958.

browser/base/content/test/static/browser_parsable_css.js | Got error message for resource://pdf.js/web/viewer.css: Unknown pseudo-class or pseudo-element ‘fullscreen’. Ruleset ignored due to bad selector. -

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

:fullscreen is an invalid pseudo-class in beta, but valid for chrome stylesheet. That should have been the case for a while. Why could a change to ::selection regress this?

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Also this whitelist item in browser_parsable_css.js:
> // PDFjs is futureproofing its pseudoselectors, and those rules are dropped.
> {sourceName: /web\/viewer\.css$/i,
>  errorMessage: /Unknown pseudo-class.*(fullscreen|selection)/i,
>  isFromDevTools: false},
should silence this error message.

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

So, the test in the try push doesn't have that whitelist item:
https://hg.mozilla.org/try/file/bed328ba3d401474c773891f7a76dcae6ac56a41/browser/base/content/test/static/browser_parsable_css.js

and that was because of the followup push in that bug: https://hg.mozilla.org/integration/mozilla-inbound/rev/2dc0a563e958

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

Unprefixed fullscreen has been enabled in nightly (but not beta), so before bug 509958, only the "selection" part was used in the whitelist item, and thus after that bug, that whitelist item doesn't catch anything and became useless, so emilio removed it.

However, unprefixed fullscreen still isn't enabled on beta and release, so without that whitelist item, this would be a problem.

I guess the fix would be to push that whitelist item conditionally according to whether unprefixed fullscreen is around.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Actually we've had a precedent for exactly the same case: https://searchfox.org/mozilla-central/rev/3f17a234769d25fca5144ebb8abc8e1cb3c56c16/browser/base/content/test/static/browser_parsable_css.js#74-80

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1460870 - Whitelist fullscreen pseudo-class in pdfjs when unprefixed fullscreen is disabled.

Review commit: https://reviewboard.mozilla.org/r/243384/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/243384/

Comment 7

[:Gijs (he/him)]

Comment on attachment 8975000 [details]
Bug 1460870 - Whitelist fullscreen pseudo-class in pdfjs when unprefixed fullscreen is disabled.

https://reviewboard.mozilla.org/r/243384/#review249238

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/87df7799c295
Whitelist fullscreen pseudo-class in pdfjs when unprefixed fullscreen is disabled. r=Gijs

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 62, though this will likely be fixed on m-c before it comes up again in triage.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/87df7799c295

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Verified fixed in today's beta simulation: https://treeherder.mozilla.org/#/jobs?repo=try&revision=89fdae9f4f5e6dbcf0abb820fd04b6cdaf3c953f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable