1280642 - Name of 'Subscribe' and 'Share This Page' tools are greyed out if moved to panel and still in customization mode

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Bogdan Maris, Desktop QA]

Attachment: Screenshot showing the issue

[Affected versions]:
- Firefox 48 beta 1
- latest Developer Edition 49.0a2
- latest Nightly 50.0a1

[Affected platforms]:
- Ubuntu 16.04 32-bit
- Windows 8.1 64-bit
- Mac OS X 10.10.5

[Steps to reproduce]:
1. Start Firefox
2. Enter Customization
3. Move 'Subscribe' and 'Share This Page' tools to panel (don't exit customization)

[Expected result]:
- Name of all tools are properly displayed

[Actual result]:
- Name of 'Subscribe' and 'Share This Page' tools are greyed out during customization

[Regression range]:
- This is not a regression range, this reproduces back to Nightly 35.0a1 where Share This Page was enabled and even further back for Subscribe icon.

[Additional notes]:
- Not sure if this is a real bug or not, but I don't see the reason to grey out the name of those tools while they are still in customization. It makes sense to grey them out if they are not available while one is visiting a specific webpage.

Comment 1

[:Gijs (he/him)]

I *think* this can be fixed by adding something like:

/* Force the color of disabled toolbar items to not be grey while in customize mode */
toolbarpaletteitem toolbarbutton[disabled] {
  color: inherit;
}

in customizeMode.inc.css ( above http://searchfox.org/mozilla-central/rev/970569ad57ac4436ff31aa2ac63f38ed3ee2932d/browser/themes/shared/customizableui/customizeMode.inc.css#228 would probably be a good place ).

Comment 2

[[:Towkir] Ahmed]

Okay, I will submit a patch soon.

Comment 3

[[:Towkir] Ahmed]

Hello, Gijs
I found something: 
file : customizeMode.inc.css
Line: 295
code:
#wrapper-edit-controls > #edit-controls > toolbarbutton > .toolbarbutton-icon {
  opacity: 1; /* To ensure these buttons always look enabled in customize mode */
}

I think these lines where written to do what we are trying to do. but I am not sure why is it not working.

Comment 4

[:Gijs (he/him)]

(In reply to [:Towkir] Ahmed from comment #3)
> Hello, Gijs
> I found something: 
> file : customizeMode.inc.css
> Line: 295
> code:
> #wrapper-edit-controls > #edit-controls > toolbarbutton >
> .toolbarbutton-icon {
>   opacity: 1; /* To ensure these buttons always look enabled in customize
> mode */
> }
> 
> I think these lines where written to do what we are trying to do. but I am
> not sure why is it not working.

They apply to different buttons, and only to the icon. What needs to change here is the text color, see comment #1. You might need to add !important - at least on OS X, the rule that makes the text grey-ish is !important ( http://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/toolbarbutton.css#32 )

Comment 5

[[:Towkir] Ahmed]

Attachment: fixedlabelcolor.patch

Hi Gijs, comment 1 is the actual solution, thanks :)

Comment 6

[Rohit Kumar Jena]

I'm new to this place and I am looking for a place to start. Can I be assigned this bug? 
Thanks.

Comment 7

[:Gijs (he/him)]

(In reply to Rohit Kumar Jena from comment #6)
> I'm new to this place and I am looking for a place to start. Can I be
> assigned this bug? 
> Thanks.

Unfortunately Towkir/Ahmed is already working on this bug. Perhaps you could consider bug 1214284?

(In reply to [:Towkir] Ahmed from comment #5)
> Created attachment 8765013 [details] [diff] [review]
> fixedlabelcolor.patch
> 
> Hi Gijs, comment 1 is the actual solution, thanks :)

Thanks! I'll review the patch tomorrow.

Comment 8

[:Gijs (he/him)]

Comment on attachment 8765013 [details] [diff] [review]
fixedlabelcolor.patch

As I noted in comment 4, you need to add !important to the rule for it to work on OS X. Could you also update the commit message to include the reviewer (suffix with ", r?gijs" or similar) and provide an updated patch with those two things addressed ? Thank you!

Comment 9

[[:Towkir] Ahmed]

should it be r?gijs or r=gjis   ?

Comment 10

[[:Towkir] Ahmed]

Attachment: fixedlabelcolorupdated.patch

Here is the updated patch with !important added and r?gijs set

Comment 11

[:Gijs (he/him)]

Comment on attachment 8765565 [details] [diff] [review]
fixedlabelcolorupdated.patch

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

Thanks!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/9676dc9d8b43
Fixed Label color for 'Subscribe' and 'Share This Page' while in customization mode. r=gijs

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9676dc9d8b43

Comment 14

[Hossain Al Ikram [:ikram] (QA Contact)]

I have reproduced this bug with Firefox Release 47.0.1 ( 2016-06-23 ) on Windows 7, 64 Bit.

The Bug's fix is now verified on Latest Nightly 50.0a1.

Build ID 	20160712030234
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0