Closed Bug 966694 Opened 8 years ago Closed 8 years ago

Quit button in AppMenu footer has no tooltip on Windows

Categories

(Firefox :: Toolbars and Customization, defect)

All
Windows 8.1
defect
Not set
normal

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)

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