973191 - DevTools themes - In sidebar dock mode, some splitters are dark in light theme

Status: VERIFIED FIXED

(DevTools :: Framework, defect)

Description

[Tim Nguyen :ntim]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140213030201

Comment 1

[Tim Nguyen :ntim]

The affected tools are :
- Inspector 
- Style Editor (you should make the toolbox small to see the issue)
- Network monitor
- Console

Comment 2

[Brian Grinstead [:bgrins]]

This seems to be an issue only when the toolbox is narrow enough to force vertical layout.

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: side-splitter-responsive.patch

I've moved the responsive styling into widgets.inc.css and away from common.css (as in Bug 957663).  Also setting the border colors in each theme next to the other splitter colors instead of using black in both cases.

The problem can be seen when shrinking the toolbox horizontal size until it forces vertical layout.  In the light theme the splitter will be black in this case (it should be #aaa).  With this patch applied, the color should be correct.

Comment 4

[Victor Porof [:vporof][:vp]]

Comment on attachment 8380663 [details] [diff] [review]
side-splitter-responsive.patch

Review of attachment 8380663 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/shared/devtools/widgets.inc.css
@@ +15,5 @@
>  .generic-toggled-side-pane[animated] {
>    transition: margin 0.25s ease-in-out;
>  }
>  
> +/* Responsive container */

I assume moving this is based on the fact that common.css is included in browser, and we don't need it there.

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Victor Porof [:vporof][:vp] from comment #4)
> Comment on attachment 8380663 [details] [diff] [review]
> side-splitter-responsive.patch
> 
> Review of attachment 8380663 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/shared/devtools/widgets.inc.css
> @@ +15,5 @@
> >  .generic-toggled-side-pane[animated] {
> >    transition: margin 0.25s ease-in-out;
> >  }
> >  
> > +/* Responsive container */
> 
> I assume moving this is based on the fact that common.css is included in
> browser, and we don't need it there.

Yes, exactly.  The devtools-responsive-container is only loaded inside some of the tools xul, never used inside of the browser.xul: https://mxr.mozilla.org/mozilla-central/search?find=%2Fbrowser%2F&string=devtools-responsive-container.

Comment 6

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/7fd003a98b55
https://tbpl.mozilla.org/?tree=Fx-Team&rev=7fd003a98b55

Comment 7

[Tim Nguyen :ntim]

Brian, thanks for the fix ! Make sure style editor is also fixed, because it uses a different splitter (thicker).

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7fd003a98b55