We should remove CSS preprocessor usage within the CSS directory, which would allow an easier path forward towards reloading the frontend without a rebuild. One option for dealing with OS-specific styles would be to apply an attribute on the root for different OSes from within theme-switching.js or similar, then those styles could be targeted using a :root[os="win"], :root[os="linux"], :root[os="osx"] or similar. If we have any styles that absolutely need to be preprocessed anyway for some reason, we should try to contain them within a single file separate from the main styling.

This patch removes a bunch of preprocessed css. I had to keep some preprocessed as they are being pulled from browser/ and do not have theme-switching.js evaluated in their scope :/ Same thing for webide.css, we don't evaluate theme-switching in it. And also need to remove the star in jar.mn. I would like to move forward with followups here as we can already cleanup a lot of %if everywhere!

Comment on attachment 8683699 [details] [diff] [review] patch v1 Review of attachment 8683699 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/themes/ruleview.css @@ +49,1 @@ > margin-top: 4px; Looks like you need to remove this for the base case.

Comment on attachment 8683699 [details] [diff] [review] patch v1 I'm going to rebase on top of bug 1183280 once it lands. ntim wrote similar patch to this, but with more cleanups. I just have splitview.css cleanup on top of his patch.

Rebased. I fixed the floating scrollbars issue. There is some namespace/:root issue with xul documents. It seems to work fine with a "xul" named namespace. This patch conflicts with bug 1196786, but I would like to see the preprocessing being removed sooner than later. This patch is much simplier and just rewrite the existing preprocessed rules by using :root[platform=""]. Hopefully ntim can continue and cleanup our CSS from unecessary rules...

Fix missing namespace in floating-scrollbar-light.css

Comment on attachment 8702589 [details] [diff] [review] patch v3 Review of attachment 8702589 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for working on this ! ::: devtools/client/themes/common.css @@ +18,1 @@ > --monospace-font-family: monospace; bgrins asked me to undo this change in https://bugzilla.mozilla.org/show_bug.cgi?id=1196786#c9 , since this file is also loaded in browser.xul (although I think it's only for the toolbox splitter and iframe) ::: devtools/client/themes/floating-scrollbars.css @@ +13,1 @@ > border: 0px solid transparent; Can you change this to border: none ? @@ +26,5 @@ > min-height: 10px; > max-height: 10px; > } > > +:root[platform="mac"] xul|slider { Are you sure it works on all docs without *|*:root[platform="mac"] ? (I haven't tested) Also see bug 1196786 comment 9 and comment 10 on how to test this floating scrollbar stuff. ::: devtools/client/themes/widgets.css @@ +355,5 @@ > > /* SideMenuWidget container */ > > .theme-dark .side-menu-widget-container:-moz-locale-dir(ltr), > .theme-dark .side-menu-widget-empty-text:-moz-locale-dir(ltr) { You should remove .theme-dark, since this rule should now apply to all themes (now that you've converted this to CSS vars) @@ +361,4 @@ > } > > .theme-dark .side-menu-widget-container:-moz-locale-dir(rtl), > .theme-dark .side-menu-widget-empty-text:-moz-locale-dir(rtl) { Same here

I moved floating scrollbar and common.css modifications to bug 1235780 and bug 1235781. And I tried to addressed all comments.

Comment on attachment 8702883 [details] [diff] [review] patch v4 Review of attachment 8702883 [details] [diff] [review]: ----------------------------------------------------------------- Nice work on splitview.css and widgets.css! The code changes look good to me, although I haven't tested this locally across all platforms

Try push with rebased patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b23db8398c39

https://hg.mozilla.org/integration/fx-team/rev/eae9f44f7444407a28ab3b876032eceebe19310c Bug 1183280 - Remove most devtools preprocessing rule in CSS files. r=bgrins

[bugday-20160323] Status: RESOLVED,FIXED -> UNVERIFIED Comments: STR: Not clear. Developer specific testing Component: Name Firefox Version 46.0b4 Build ID 20160322075646 Update Channel beta User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 OS Windows 7 SP1 x86_64 Expected Results: Developer specific testing expected Actual Results: As expected