1296395 - Remove unnecessary #addon-bar rules from /browser/themes/

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

The #addon-bar is always hidden, but we have some rules in /browser/themes that specify properties that only make sense if the element is visible.

Comment 1

[Deepjyoti Mondal]

Hi, I would like to work on this bug, So all that needs to be done is remove the #addon-bar rules from all the css files present under /browser/themes right?

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Not necessarily all of them. You will have to use your judgement to determine if they are needed. The addon-bar should remain hidden, so don't remove anything that hides it, but you can remove everything else that would style it.

I usually wait to assign the bug until a patch has been uploaded to the bug.

Comment 3

[Glenn Fernandes [:torroid]]

Attachment: Removed unnecessary styling for hidden addon bar

I hope its the right file and the right place.
I had a doubt whether to keep the transition or not, I've removed it though.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8783022 [details] [diff] [review]
Removed unnecessary styling for hidden addon bar

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

Thanks for the patch. This is not going to work right though.

In this rule,
> #navigator-toolbox > toolbar:not(#toolbar-menubar):not(#TabsToolbar):not(#nav-bar):not(#addon-bar) {	
>   overflow: -moz-hidden-unscrollable;	
>   max-height: 4em;	
>   transition: min-height 170ms ease-out, max-height 170ms ease-out;	
> }

these three properties are applied to all toolbars with the exception of the menubar, tab strip, nav-bar, and addon-bar. This is the right thing to do because we're not applying these styles to the addon-bar.

However, in this rule,
> #navigator-toolbox > toolbar:not(#toolbar-menubar):not(#TabsToolbar) {
>   background-clip: padding-box;
>   background-image: linear-gradient(@toolbarHighlight@, @toolbarHighlight@);
> }

We are applying a linear-gradient to the addon-bar which is unnecessary. This selector can be changed to:
> #navigator-toolbox > toolbar:not(#toolbar-menubar):not(#TabsToolbar):not(#addon-bar) {
>   background-clip: padding-box;
>   background-image: linear-gradient(@toolbarHighlight@, @toolbarHighlight@);
> }

for example.

Comment 5

[Glenn Fernandes [:torroid]]

Attachment: Removed unnecessary #addon-bar rules from /browser/themes/

I hope this is right.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8784377 [details] [diff] [review]
Removed unnecessary #addon-bar rules from /browser/themes/

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

Looks good, just one more tweak. Can you also update your commit message to have the bug number at the beginning and put r=jaws at the end?
"Bug 1296395 - Remove unnecessary styling for hidden addon bar. r=jaws"

::: browser/themes/linux/browser.css
@@ +67,4 @@
>    background-image: linear-gradient(@toolbarHighlight@, @toolbarHighlight@);
>  }
>  
> +#navigator-toolbox > toolbar:not(#toolbar-menubar):not(#TabsToolbar):-moz-lwtheme:not(#addon-bar) {

We usually put psuedo-classes at the end of the selector, so this should be:
#navigator-toolbox > toolbar:not(#toolbar-menubar):not(#TabsToolbar):not(#addon-bar):-moz-lwtheme

Comment 7

[Glenn Fernandes [:torroid]]

Attachment: Removed unnecessary styling for hidden addon bar.

Changed the commit message and put the pseudo-classes at the end of the selector.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8785360 [details] [diff] [review]
Removed unnecessary styling for hidden addon bar.

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

Looks good, thanks and congratulations on fixing your first bug in Firefox! I'll mark this as checkin-needed, and in a day or two it should get checked in to the Mozilla source code repository. A couple days after that the changes should appear in Firefox Nightly. I'll see if I can find another bug that you can work on :)

Comment 9

[Glenn Fernandes [:torroid]]

Thank you! I'm glad I could fix it. I'd definitely like to work on another bug.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/47da2b6ea281
Remove unnecessary styling for hidden addon bar. r=jaws

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/47da2b6ea281