This is in use in the sources list in the debugger, shader editor, most of the network monitor, and the profiles list in the profiler. There seems to be some concept of manual themes for this in widgets.css already, so it may not be too big of a deal to switch this over to use theme-body and existing colors.

Victor, if you would be interested in starting on this it would definitely increase the odds of it landing before the next uplift. If not, I will swing back around and get to it after a few of the others I have in progress.

We may want to merge all three widgets.css files together into a shared file to make theming this (and the breadcrumbs) easier.

That's the plan.

WIP attempt at this. Uses only colors from https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors and documents them, so when we switch to CSS variables our lives will be easier.

Nice! :D

Woohoo! I assume this will fix bug 931485 as well?

(In reply to Panos Astithas [:past] from comment #9) > Woohoo! I assume this will fix bug 931485 as well? Yes, I'm going to dupe it.

(In reply to Victor Porof [:vp] from comment #7) > Created attachment 8350517 [details] > light screenshot nice.

I think this is pretty much final. I'll keep this patch in my queue and play with the tools for a while to make sure there's no hideousness anywhere, then ask for r+. This depends on bug 951633 anyway.

I'm happy with this.

Making this a P1 mainly just because it blocks a bunch of other bugs.

Comment on attachment 8355789 [details] [diff] [review] v3 Review of attachment 8355789 [details] [diff] [review]: ----------------------------------------------------------------- I've looked through all the non-CSS changes, and everything looks fine except that there is a new folder where the shared images should go. Will review the CSS changes next. ::: browser/themes/linux/jar.mn @@ +202,5 @@ > * skin/classic/browser/devtools/scratchpad.css (devtools/scratchpad.css) > skin/classic/browser/devtools/magnifying-glass.png (devtools/magnifying-glass.png) > skin/classic/browser/devtools/option-icon.png (devtools/option-icon.png) > skin/classic/browser/devtools/itemToggle.png (devtools/itemToggle.png) > + skin/classic/browser/devtools/itemArrow-dark-rtl.png (../shared/devtools/itemArrow-dark-rtl.png) Can you move these images and their references into the new ../shared/devtools/images folder?

bokay!

Comment on attachment 8355789 [details] [diff] [review] v3 Review of attachment 8355789 [details] [diff] [review]: ----------------------------------------------------------------- The patch is failing to apply on latest - can you rebase it?

Did you apply the patch in bug 951795?

(In reply to Victor Porof [:vp] from comment #19) > Did you apply the patch in bug 951795? No, I hadn't - everything is applying now.

Try builds: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/vporof@mozilla.com-43d2259a5709/

Bugzilla!

Light theme screenshot of side menu widget in Ubuntu, OSX, and Windows environments

Dark theme screenshot of side menu widget in Ubuntu, OSX, and Windows environments

Comment on attachment 8355789 [details] [diff] [review] v3 Review of attachment 8355789 [details] [diff] [review]: ----------------------------------------------------------------- Looking at the result in Windows and Linux, I see an issue with the add watch expression box in the debugger. Besides this, everything that I've checked looks good. ::: browser/devtools/debugger/debugger.xul @@ -422,5 @@ > <tab id="variables-tab" label="&debuggerUI.tabs.variables;"/> > <tab id="events-tab" label="&debuggerUI.tabs.events;"/> > </tabs> > <tabpanels flex="1"> > - <tabpanel id="variables-tabpanel" class="theme-body"> Removing the theme-body class here seems to be causing the white background on the 'add watch expression' box as seen in Attachment 8356151 [details] in Windows and Linux (they are inheriting the white background from the <tabpanels> element, and having theme-body here sets the background color as expected). This may also be able to be accomplished by setting background transparent on devtools-sidebar-tabs > tabpanels in toolbars.inc.css, but in general there shouldn't be much harm in having theme-body defined twice (since it just defines text and background color).

(In reply to Brian Grinstead [:bgrins] from comment #25) I actually think it'd be better for this to be fixed at the toolbars.inc.css level, since this is a specific problem with the tabpanels on a specific platform, not the theme-body colors themselves not being applied as necessary. Setting a transparent background on the tabpanels will avoid having to specify the theme-body class in too many places in the future as well. What do you think? Do you feel strongly against this?

(In reply to Victor Porof [:vp] from comment #26) > (In reply to Brian Grinstead [:bgrins] from comment #25) > > I actually think it'd be better for this to be fixed at the toolbars.inc.css > level, since this is a specific problem with the tabpanels on a specific > platform, not the theme-body colors themselves not being applied as > necessary. Setting a transparent background on the tabpanels will avoid > having to specify the theme-body class in too many places in the future as > well. > > What do you think? Do you feel strongly against this? I don't feel strongly about it, it seems reasonable to try and fix it in toolbars.inc.css. As long as we double check the other places using this same devtools-sidebar-tabs element to make sure that they are not negatively affected. Seems that this class is used in debugger, inspector, netmonitor, scratchpad, and webconsole.

Fixed tabpanels being transparent. Everything looks good as far as I can tell. Here's a try run: https://tbpl.mozilla.org/?tree=Try&rev=706261d4fcfc

Comment on attachment 8357096 [details] [diff] [review] v4 Review of attachment 8357096 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me - issue with watch expressions is resolved on Windows and Linux

Nice work ! But shouldn't the gradient be gone ? Stephen's mockups makes the UI flat so...

Btw, style editor still has old look unfortunatly.

(In reply to Tim Nguyen [:ntim] from comment #34) > Btw, style editor still has old look unfortunatly. That's because the Style Editor doesn't use the widget unfortunately, but an older "splitview" implementation. There was a bug about switching, but I can't find it right now.

(In reply to Victor Porof [:vp] from comment #35) > (In reply to Tim Nguyen [:ntim] from comment #34) > > Btw, style editor still has old look unfortunatly. > > That's because the Style Editor doesn't use the widget unfortunately, but an > older "splitview" implementation. There was a bug about switching, but I > can't find it right now. The Style Editor sidebar styles will be updated in Bug 957117.