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)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 52
People
(Reporter: xidorn, Assigned: florian)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•9 years ago
|
||
Also, the appDir should get 'GreD' instead of 'XCurProcD'.
Assignee | ||
Comment 2•8 years ago
|
||
Assignee | ||
Comment 3•8 years ago
|
||
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
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → florian
Assignee | ||
Comment 4•8 years ago
|
||
Assignee | ||
Comment 5•8 years ago
|
||
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 6•8 years ago
|
||
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+
Assignee | ||
Comment 7•8 years ago
|
||
(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.
Assignee | ||
Comment 8•8 years ago
|
||
Assignee | ||
Comment 9•8 years ago
|
||
Assignee | ||
Comment 10•8 years ago
|
||
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.
Comment 11•8 years ago
|
||
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)
Comment 12•8 years ago
|
||
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
Assignee | ||
Comment 13•8 years ago
|
||
(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)
Assignee | ||
Comment 14•8 years ago
|
||
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.
Comment 15•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in
before you can comment on or make changes to this bug.
Description
•