Closed Bug 1221383 Opened 9 years ago Closed 8 years ago

Make browser_parsable_css.js test cover all CSS files we are shipping

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox45 --- affected
firefox52 --- fixed

People

(Reporter: xidorn, Assigned: florian)

References

Details

Attachments

(1 file)

Currently, browser_parsable_css.js test covers only the CSS files inside browser/ directory. We may want to extend the range to all CSS files we are shipping, including those in the chrome/ directory and webapprt/ directory. If we are going to do so, there are several things we need to do: * Handle UA-only things including some properties and pseudo-elements/classes. This probably can be done via the trick used for injecting devtools/server/actors/highlighters.css. * Properly register all necessary manifests. The manifests inside webapprt/ are not registered by default. We would need to register them before testing. To make this work with packaged build, we may need to add a method to nsIComponentRegistrar which calls XRE_AddJarManifestLocation instead of XRE_AddManifestLocation, so that we can register files inside omni.ja.
Also, the appDir should get 'GreD' instead of 'XCurProcD'.
Here is the output from the try run: All platforms error message for chrome://mozapps/skin/aboutNetworking.css: Error in parsing value for ‘border’. Declaration dropped. error message for chrome://mozapps/skin/extensions/newaddon.css: Error in parsing value for ‘-moz-box-pack’. Declaration dropped. - missing chrome://global/skin/columnselect.gif referenced from chrome://global/skin/menu.css Linux missing chrome://global/skin/progressmeter/progressmeter-busy.gif referenced from chrome://global/skin/tree.css missing chrome://global/skin/scale/scale-tray-horiz.gif referenced from chrome://global/skin/scale.css missing chrome://global/skin/scale/scale-tray-vert.gif referenced from chrome://global/skin/scale.css missing chrome://mozapps/skin/update/update.png referenced from chrome://mozapps/skin/extensions/update.css missing chrome://mozapps/skin/extensions/rating-unrated.png referenced from chrome://mozapps/skin/extensions/extensions.css Mac missing chrome://global/skin/filepicker/dir-closed.gif referenced from chrome://global/skin/filepicker.css missing chrome://global/skin/filepicker/blank.gif referenced from chrome://global/skin/filepicker.css missing chrome://global/skin/filepicker/folder-up.gif referenced from chrome://global/skin/filepicker.css missing chrome://global/skin/filepicker/folder-home.gif referenced from chrome://global/skin/filepicker.css missing chrome://mozapps/skin/update/warning.gif referenced from chrome://mozapps/skin/extensions/update.css Windows missing chrome://global/skin/progressmeter/progressmeter-busy.gif referenced from chrome://global/skin/tree.css missing chrome://global/skin/icons/notfound.png referenced from chrome://mozapps/skin/update/updates.css missing chrome://global/skin/scale/scale-tray-horiz.gif referenced from chrome://global/skin/scale.css missing chrome://global/skin/scale/scale-tray-vert.gif referenced from chrome://global/skin/scale.css missing chrome://mozapps/skin/update/update.png referenced from chrome://mozapps/skin/extensions/update.css
Depends on: 1303353
Depends on: 1303355
Depends on: 1303356
Depends on: 1305075
Depends on: 1305662
Depends on: 1305701
Depends on: 1305724
Assignee: nobody → florian
Depends on: 1305748
Depends on: 1305764
Attached patch Patch (deleted) — Splinter Review
Only bug 1303355 is left in the dependencies, and someone has started working on it, so I think it's time to request review here :-).
Attachment #8798380 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8798380 [details] [diff] [review] Patch Review of attachment 8798380 [details] [diff] [review]: ----------------------------------------------------------------- r=me ::: browser/base/content/test/general/browser_parsable_css.js @@ +31,5 @@ > {sourceName: /responsive-ua\.css$/i, > errorMessage: /Unknown pseudo-class.*moz-dropdown-list/i, > isFromDevTools: true}, > + > + {sourceName: /(contenteditable|EditorOverride|svg|forms|html|mathml|ua)\.css$/i, This should probably have a \b or \/ before the group to avoid substring matching other stylesheets. Same for the html/mathml/ua one below. @@ +32,5 @@ > errorMessage: /Unknown pseudo-class.*moz-dropdown-list/i, > isFromDevTools: true}, > + > + {sourceName: /(contenteditable|EditorOverride|svg|forms|html|mathml|ua)\.css$/i, > + errorMessage: /Unknown pseudo-class.*-moz-/i, This is quite broad. Why is this necessary, specifically? Why are we declaring all these styles that we don't understand - is it just because these are UA sheets and some of their styles don't apply when used as a content sheet? Can we fix this by parsing them as UA sheets somehow (maybe as a followup bug) ?
Attachment #8798380 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs Kruitbosch from comment #6) > ::: browser/base/content/test/general/browser_parsable_css.js > @@ +31,5 @@ > > {sourceName: /responsive-ua\.css$/i, > > errorMessage: /Unknown pseudo-class.*moz-dropdown-list/i, > > isFromDevTools: true}, > > + > > + {sourceName: /(contenteditable|EditorOverride|svg|forms|html|mathml|ua)\.css$/i, > > This should probably have a \b or \/ before the group to avoid substring > matching other stylesheets. Same for the html/mathml/ua one below. Good idea, will do. > @@ +32,5 @@ > > errorMessage: /Unknown pseudo-class.*moz-dropdown-list/i, > > isFromDevTools: true}, > > + > > + {sourceName: /(contenteditable|EditorOverride|svg|forms|html|mathml|ua)\.css$/i, > > + errorMessage: /Unknown pseudo-class.*-moz-/i, > > This is quite broad. Why is this necessary, specifically? Why are we > declaring all these styles that we don't understand - is it just because > these are UA sheets and some of their styles don't apply when used as a > content sheet? Yes, it's stuff that only makes sense in UA sheets, here is the list of errors ignored by this exception: chrome/toolkit/res/forms.css: -moz-button-content chrome/toolkit/res/forms.css: -moz-display-comboboxcontrol-frame chrome/toolkit/res/forms.css: -moz-dropdown-list chrome/toolkit/res/forms.css: -moz-fieldset-content chrome/toolkit/res/forms.css: -moz-number-spin-box chrome/toolkit/res/forms.css: -moz-number-spin-down chrome/toolkit/res/forms.css: -moz-number-spin-up chrome/toolkit/res/forms.css: -moz-number-text chrome/toolkit/res/forms.css: -moz-number-wrapper chrome/toolkit/res/html.css: -moz-html-canvas-content chrome/toolkit/res/html.css: -moz-native-anonymous chrome/toolkit/res/html.css: -moz-suppressed chrome/toolkit/res/html.css: -moz-user-disabled chrome/toolkit/res/ua.css: -moz-anonymous-block chrome/toolkit/res/ua.css: -moz-anonymous-flex-item chrome/toolkit/res/ua.css: -moz-anonymous-positioned-block chrome/toolkit/res/ua.css: -moz-browser-frame chrome/toolkit/res/ua.css: -moz-column-content chrome/toolkit/res/ua.css: -moz-inline-table chrome/toolkit/res/ua.css: -moz-native-anonymous chrome/toolkit/res/ua.css: -moz-page chrome/toolkit/res/ua.css: -moz-page-sequence chrome/toolkit/res/ua.css: -moz-pagebreak chrome/toolkit/res/ua.css: -moz-pagecontent chrome/toolkit/res/ua.css: -moz-ruby chrome/toolkit/res/ua.css: -moz-ruby-base chrome/toolkit/res/ua.css: -moz-ruby-base-container chrome/toolkit/res/ua.css: -moz-ruby-text chrome/toolkit/res/ua.css: -moz-ruby-text-container chrome/toolkit/res/ua.css: -moz-scrolled-content chrome/toolkit/res/ua.css: -moz-table chrome/toolkit/res/ua.css: -moz-table-cell chrome/toolkit/res/ua.css: -moz-table-column chrome/toolkit/res/ua.css: -moz-table-column-group chrome/toolkit/res/ua.css: -moz-table-row chrome/toolkit/res/ua.css: -moz-table-row-group chrome/toolkit/res/ua.css: -moz-table-wrapper chrome/toolkit/res/ua.css: -moz-viewport chrome/toolkit/res/ua.css: -moz-viewport-scroll chrome/toolkit/res/ua.css: -moz-xul-anonymous-block editor/composer/res/EditorOverride.css: -moz-canvas editor/composer/res/EditorOverride.css: -moz-display-comboboxcontrol-frame layout/mathml/mathml.css: -moz-mathml-anonymous-block layout/style/contenteditable.css: -moz-canvas layout/style/contenteditable.css: -moz-display-comboboxcontrol-frame layout/svg/svg.css: -moz-svg-foreign-content layout/svg/svg.css: -moz-svg-marker-anon-child layout/svg/svg.css: -moz-svg-text layout/svg/svg.css: -moz-viewport > Can we fix this by parsing them as UA sheets somehow (maybe > as a followup bug) ? I can file a bug if you like, but I don't see right now how we would handle parsing as UA sheets.
https://hg.mozilla.org/integration/fx-team/rev/12a9cca092f816fabb67cc8a139e0cbd5b1cea37 Bug 1221383 - Make browser_parsable_css.js test cover all CSS files we are shipping, r=Gijs.
sorry had to back this out for frequent failures like https://treeherder.mozilla.org/logviewer.html#?job_id=11953796&repo=fx-team
Flags: needinfo?(florian)
Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/c63097845cfb Backed out changeset 12a9cca092f8 for frequent failures in browser_parsable_css.js
(In reply to Carsten Book [:Tomcat] from comment #11) > sorry had to back this out for frequent failures like > https://treeherder.mozilla.org/logviewer.html#?job_id=11953796&repo=fx-team I attached the fix in bug 1307445.
Flags: needinfo?(florian)
https://hg.mozilla.org/integration/fx-team/rev/0342df5d06e1f718155d6d754c25aae5f8676764 Bug 1221383 - Make browser_parsable_css.js test cover all CSS files we are shipping, r=Gijs.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: