Closed
Bug 978491
Opened 10 years ago
Closed 10 years ago
"Sidebars" widget has no inverted icon in subview on Windows and Linux
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
RESOLVED
FIXED
Firefox 31
Tracking | Status | |
---|---|---|
firefox29 | --- | unaffected |
firefox30 | --- | fixed |
firefox31 | --- | verified |
People
(Reporter: u428464, Assigned: shorlander)
References
(Blocks 1 open bug)
Details
(Keywords: regression, Whiteboard: [Australis:P3-])
Attachments
(1 file, 1 obsolete file)
54.72 KB,
patch
|
mikedeboer
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
The new sidebars widget has no inverted icon when the subview is shown in the panel. It should for consistency.
Blocks: australis-cust
Whiteboard: [Australis:P4]
Updated•10 years ago
|
Blocks: 960198
Whiteboard: [Australis:P4] → [Australis:P3-][aurora-unaffected]
Comment 1•10 years ago
|
||
Fixed by backing out the sidebars widget ( bug 960198)
Assignee: nobody → jaws
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
status-firefox29:
--- → unaffected
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
The issue is back in latest Nightlies.
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---
Comment 3•10 years ago
|
||
The icon is missing from the spritesheet on Windows and Linux. I should have caught this when reviewing 960198 (unless they were removed by one of the other changes...) Shane?
Assignee: jaws → nobody
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mixedpuppy)
Whiteboard: [Australis:P3-][aurora-unaffected] → [Australis:P3-]
I think it may have been caused by bug 969904.
Comment 5•10 years ago
|
||
(In reply to Guillaume C. [:ge3k0s] from comment #4) > I think it may have been caused by bug 969904. Indeed: https://hg.mozilla.org/mozilla-central/raw-file/02be46ac2b55/browser/themes/windows/menuPanel-aero.png https://hg.mozilla.org/mozilla-central/raw-file/80fda301471e/browser/themes/windows/menuPanel-aero.png Stephen, Mike? I also noticed in this (somewhat 'dumb') visual diff that the find icon was reversed. ISTR that was discussed on IRC and *wasn't* intended? Maybe I misunderstood the conversation at the time... (note that it seems like the icon was changed to have the sidebar separator further from the right, which means we can't just put back the part of the icon that was deleted. :-( )
Flags: needinfo?(shorlander)
Flags: needinfo?(mixedpuppy)
Flags: needinfo?(mdeboer)
Keywords: regression
Summary: "Sidebars" widget has no inverted icon in subview → "Sidebars" widget has no inverted icon in subview on Windows and Linux
Updated•10 years ago
|
Comment 6•10 years ago
|
||
I didn't catch the missing inverted icon for the sidebars widget. It should be re-added! The find button should be inverted for windows, compared with OSX, at least that's what Stephen told me. If this is wrong at the moment, then this would mean I checked in the wrong patch and that would make me super sad.
Flags: needinfo?(mdeboer)
Updated•10 years ago
|
Assignee: nobody → shorlander
Status: NEW → ASSIGNED
Flags: needinfo?(shorlander)
Comment 7•10 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #6) > The find button should be inverted for windows, compared with OSX, at least > that's what Stephen told me. If this is wrong at the moment, then this would > mean I checked in the wrong patch and that would make me super sad. As much as I hate being the bearer of bad news: http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/menuPanel.png http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/menuPanel.png have it pointing the same direction.
Comment 8•10 years ago
|
||
Yes, it seemed like Stephen didn't update the menuPanel.png resource properly and I didn't check thoroughly enough, which I should've done. Stephen, can you post a corrected menuPanel.png in this bug please? I can produce/ review the patch if you'd like. Thanks!
Updated•10 years ago
|
Flags: needinfo?(shorlander)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8393702 -
Flags: review?(mdeboer)
Flags: needinfo?(shorlander)
Updated•10 years ago
|
Attachment #8393702 -
Flags: review?(mdeboer) → review+
Updated•10 years ago
|
Keywords: checkin-needed
Comment 10•10 years ago
|
||
If the find icon is inverted in menu panel, it'd be nice if the toolbar icon was too (for consistency). It's not currently the case.
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to Tim Nguyen [:ntim] from comment #10) > If the find icon is inverted in menu panel, it'd be nice if the toolbar icon > was too (for consistency). It's not currently the case. Yes, would be nice for the toolbar also. I don't think it currently has a toggled state attribute though.
Comment 12•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/970e6edabe39
status-firefox30:
--- → affected
status-firefox31:
--- → affected
Keywords: checkin-needed
Target Milestone: Firefox 30 → ---
Comment 13•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/970e6edabe39
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Updated•10 years ago
|
Comment 14•10 years ago
|
||
Is it on Aurora ?
Comment 15•10 years ago
|
||
Comment on attachment 8393702 [details] [diff] [review] add-inverted-sidebars-icon.patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 969904 User impact if declined: No inverted icon for sidebar's subview Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): Low, small image updating String or IDL/UUID changes made by this patch: None
Attachment #8393702 -
Flags: approval-mozilla-aurora?
Comment 16•10 years ago
|
||
Tim, please leave requesting uplift to the patch author or reviewer. Not all patches can be uplifted without changes and it is better for the author/reviewer to coordinate uplift.
Updated•10 years ago
|
Attachment #8393702 -
Flags: approval-mozilla-aurora?
Comment 17•10 years ago
|
||
Comment on attachment 8393702 [details] [diff] [review] add-inverted-sidebars-icon.patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 969904 User impact if declined: No inverted icon for sidebar's subview Testing completed (on m-c, etc.): landed on m-c, looking a-ok! Risk to taking this patch (and alternatives if risky): minor String or IDL/UUID changes made by this patch: n/a
Attachment #8393702 -
Flags: approval-mozilla-beta?
Attachment #8393702 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8393702 -
Flags: approval-mozilla-beta?
Updated•10 years ago
|
Attachment #8393702 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•10 years ago
|
Keywords: checkin-needed
Comment 18•10 years ago
|
||
remote: https://hg.mozilla.org/releases/mozilla-aurora/rev/175ec8a02237
Keywords: checkin-needed
Comment 19•10 years ago
|
||
Shorlander forgot to fix the problem for linux. Here's a patch that uses the Windows menuPanel.png asset for linux (as it always did before).
Attachment #8395939 -
Flags: review?(gijskruitbosch+bugs)
Updated•10 years ago
|
Attachment #8395939 -
Flags: ui-review?(shorlander)
Comment 20•10 years ago
|
||
Comment on attachment 8395939 [details] [diff] [review] Patch for linux Clearing and filing a new bug per IRC.
Attachment #8395939 -
Flags: ui-review?(shorlander)
Attachment #8395939 -
Flags: review?(gijskruitbosch+bugs)
Updated•10 years ago
|
Attachment #8395939 -
Attachment is obsolete: true
Comment 21•10 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 Tested the fix on Windows 7 64bit using Firefox 31 beta 5, build ID: 987388 The Linux fix for this issue was verified in bug 987388.
You need to log in
before you can comment on or make changes to this bug.
Description
•