Closed
Bug 966694
Opened 10 years ago
Closed 10 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•10 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•10 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•10 years ago
|
||
Hm, indeed it does. But, I think the tooltip should include brandShortName too though.
Flags: needinfo?(bmcbride)
|
|
Assignee | |
Comment 4•10 years ago
|
||
Attachment #8369141 -
Attachment is obsolete: true
Attachment #8369141 -
Flags: review?(gijskruitbosch+bugs)
Attachment #8369273 -
Flags: review?(gijskruitbosch+bugs)
|
|
||
Comment 5•10 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•10 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•10 years ago
|
Attachment #8369334 -
Flags: review?(bmcbride) → review+
|
|
||
Comment 8•10 years ago
|
||
remote: https://hg.mozilla.org/integration/fx-team/rev/0b53328bdc60
|
||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0268f681c73e https://hg.mozilla.org/mozilla-central/rev/0b53328bdc60
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P?][strings][fixed-in-fx-team] → [Australis:P?][strings]
Target Milestone: --- → Firefox 29
|
||
Updated•10 years ago
|
Whiteboard: [Australis:P?][strings] → [Australis:P?][strings][good first verify]
|
||
Comment 10•10 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
•