Closed
Bug 966694
Opened 11 years ago
Closed 11 years ago
Quit button in AppMenu footer has no tooltip on Windows
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
RESOLVED
FIXED
Firefox 29
People
(Reporter: Unfocused, Assigned: Unfocused)
References
(Blocks 1 open bug)
Details
(Whiteboard: [Australis:P?][strings][good first verify] [testday-20140328])
Attachments
(2 files, 1 obsolete file)
|
2.25 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
|
2.80 KB,
patch
|
Unfocused
:
review+
|
Details | Diff | Splinter Review |
Think this in an inadvertent side-effect of bug 940116, which (ironically) fixed the tooltips of the appmenu buttons to be more consistent.
Looking at the code, it seems OSX and Linux get a tooltip for the Quit button, since they have shortcuts for it. Windows doesn't so it misses out on *any* tooltip - which isn't ideal, given the button is just an image with no visible label.
|
Assignee | |
Comment 1•11 years ago
|
||
Ended up refactoring the preprocessor directives in _updateQuitTooltip, because I found it became too confusing once I added Windows to the mix. So, little bit of redundancy now, but so much easier to understand.
Attachment #8369141 -
Flags: review?(gijskruitbosch+bugs)
|
||
Comment 2•11 years ago
|
||
Hrm. AIUI, we were trying to match OS conventions with this tooltip... I just shouldn't have removed this line:
http://hg.mozilla.org/mozilla-central/rev/9b3dc7c6dae9#l2.24
and the idea of leaving that there was why _updateQuitTooltip didn't do anything for Windows. I then just accidentally removed too much out of there.
Does that sound acceptable? :-)
Flags: needinfo?(bmcbride)
|
|
Assignee | |
Comment 3•11 years ago
|
||
Hm, indeed it does. But, I think the tooltip should include brandShortName too though.
Flags: needinfo?(bmcbride)
|
|
Assignee | |
Comment 4•11 years ago
|
||
Attachment #8369141 -
Attachment is obsolete: true
Attachment #8369141 -
Flags: review?(gijskruitbosch+bugs)
Attachment #8369273 -
Flags: review?(gijskruitbosch+bugs)
|
|
||
Comment 5•11 years ago
|
||
Comment on attachment 8369273 [details] [diff] [review]
Patch v2
Review of attachment 8369273 [details] [diff] [review]:
-----------------------------------------------------------------
Hm. Works for me, but note that we don't do this on Linux either. Should we be doing the same there?
Attachment #8369273 -
Flags: review?(gijskruitbosch+bugs) → review+
|
|
||
Comment 6•11 years ago
|
||
Landed: remote: https://hg.mozilla.org/integration/fx-team/rev/0268f681c73e
Whiteboard: [Australis:P?][strings] → [Australis:P?][strings][fixed-in-fx-team]
|
|
Assignee | |
Updated•11 years ago
|
Attachment #8369334 -
Flags: review?(bmcbride) → review+
|
|
||
Comment 8•11 years ago
|
||
|
||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0268f681c73e
https://hg.mozilla.org/mozilla-central/rev/0b53328bdc60
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P?][strings][fixed-in-fx-team] → [Australis:P?][strings]
Target Milestone: --- → Firefox 29
|
||
Updated•11 years ago
|
Whiteboard: [Australis:P?][strings] → [Australis:P?][strings][good first verify]
|
||
Comment 10•11 years ago
|
||
Verified fixed using Windows 7 64 bit and Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
Whiteboard: [Australis:P?][strings][good first verify] → [Australis:P?][strings][good first verify] [testday-20140328]
You need to log in
before you can comment on or make changes to this bug.
Description
•