Closed Bug 956130 Opened 10 years ago Closed 10 years ago

The tab groups button has the wrong icon when it is shown in non-overflow mode on Windows

Categories

(Firefox :: Theme, defect)

All
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 29

People

(Reporter: jaws, Assigned: jaws)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P1])

Attachments

(2 files)

Attached image Screenshot of bug
See attachment.
Whiteboard: [Australis:P1]
Attached patch PatchSplinter Review
The CSS at http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser-aero.css#236 is what was setting the wrong icon here. Adding :not(#tabview-button) to this isn't right, because the primaryToolbarButtons is basically only used for setting the icon sprite but the tabview button doesn't use the sprite. Also, adding :not(#tabview-button) changes the specificity of the selector enough to make http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#1619 no longer have the most precedence.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #8355311 - Flags: review?(mconley)
Comment on attachment 8355311 [details] [diff] [review]
Patch

Yes, I think this is the right course of action here.
Attachment #8355311 - Flags: review?(mconley) → review+
https://hg.mozilla.org/mozilla-central/rev/57f666e0fdb9
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
The icon is still wrong. Although its the tab groups icon, but its the black version. not the white one.
(In reply to Girish Sharma [:Optimizer] from comment #7)
> The icon is still wrong. Although its the tab groups icon, but its the black
> version. not the white one.
Confirmed, don't know if it's good or bad.
Also, I don't see the panorama icon only after creating some groups first. What's the expected behavior here?
Flags: needinfo?(jaws)
(In reply to Girish Sharma [:Optimizer] from comment #7)
> The icon is still wrong. Although its the tab groups icon, but its the black
> version. not the white one.

It should only show the inverted (white) icon if there is a lightweight theme applied that uses bright-text.

(In reply to Paul Silaghi, QA [:pauly] from comment #8)
> Also, I don't see the panorama icon only after creating some groups first.
> What's the expected behavior here?

I'm confused by what this is saying.

Did you use a new profile, enter the Tab Groups view (Ctrl+Shift+E), create some tab groups, exit the Tab Groups view, and not see the Tab Groups icon in the toolbar? I'm not sure if we did that before.

Or did you mean something different?
Flags: needinfo?(jaws) → needinfo?(paul.silaghi)
(In reply to Jared Wein [:jaws] from comment #9)
> (In reply to Girish Sharma [:Optimizer] from comment #7)
> > The icon is still wrong. Although its the tab groups icon, but its the black
> > version. not the white one.
> 
> It should only show the inverted (white) icon if there is a lightweight
> theme applied that uses bright-text.


No It should not. The downloads button, when put in tabs toolbar , is also white. This was the behavior prior to Australis too.

> (In reply to Paul Silaghi, QA [:pauly] from comment #8)
> > Also, I don't see the panorama icon only after creating some groups first.
> > What's the expected behavior here?
> 
> I'm confused by what this is saying.
> 
> Did you use a new profile, enter the Tab Groups view (Ctrl+Shift+E), create
> some tab groups, exit the Tab Groups view, and not see the Tab Groups icon
> in the toolbar? I'm not sure if we did that before.
> 
> Or did you mean something different?

I think he means that until we create atleast 2 tab groups, we do not see the tab groups icon
(In reply to Girish Sharma [:Optimizer] from comment #10)
> I think he means that until we create atleast 2 tab groups, we do not see
> the tab groups icon
Exactly. 

> enter the Tab Groups view (Ctrl+Shift+E), create
> some tab groups, exit the Tab Groups view
This isn't a very common scenario, people don't know this shortcut (that isn't posted anywhere actually).
Why not show the icon from the beginning ?
Flags: needinfo?(paul.silaghi)
Depends on: 956994
Flags: needinfo?(jaws)
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Girish Sharma [:Optimizer] from comment #10)
> (In reply to Jared Wein [:jaws] from comment #9)
> > (In reply to Girish Sharma [:Optimizer] from comment #7)
> > > The icon is still wrong. Although its the tab groups icon, but its the black
> > > version. not the white one.
> > 
> > It should only show the inverted (white) icon if there is a lightweight
> > theme applied that uses bright-text.
> 
> 
> No It should not. The downloads button, when put in tabs toolbar , is also
> white. This was the behavior prior to Australis too.

I agree with you, but different people disagree, see bug 956491. Stephen, could you make a call here, please?
Flags: needinfo?(shorlander)
Flags: needinfo?(jaws)
Flags: needinfo?(gijskruitbosch+bugs)
Then the downloads icon should also be made dark, to be consistent.
Also, this means that we would be changing the default behavior which has been there since panorama (light themes, not the tab groups) existed.
Until Stephen decides on the final solution, can we at least have this to its original state before the regression due to Australis ?
Flags: needinfo?(gijskruitbosch+bugs)
I think the current state is livable if not releasable, so I'd rather get a decision from Stephen before touching this code more.
Flags: needinfo?(gijskruitbosch+bugs)
Answer in bug 956491
Flags: needinfo?(shorlander)
QA Contact: cornel.ionce
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Windows NT 6.1; rv:29.0) Gecko/20100101 Firefox/29.0
Build ID: 20140317004002

The tab groups button is now implemented accordingly to design.
Marking issue as verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: