Closed
Bug 892483
Opened 11 years ago
Closed 11 years ago
Zoom controls should not have text labels
Categories
(Firefox :: Toolbars and Customization, defect)
Firefox
Toolbars and Customization
Tracking
()
RESOLVED
FIXED
Firefox 28
People
(Reporter: mikedeboer, Assigned: Gijs)
References
(Blocks 1 open bug, )
Details
(Keywords: regression, Whiteboard: [Australis:M8][Australis:P1])
Attachments
(2 files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
See title and screenshot.
The labels also cause the layout to shift, which doesn't look good.
Assignee | ||
Comment 1•11 years ago
|
||
When did this regress? Also, is this really P1? I think OS X, with the window bordering on the edge of the screen, is the only situation where the labels get cut off. At least on Windows, they looked fine when I checked yesterday... I was under the impression the labels had always been there.
Comment 2•11 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #1)
> When did this regress? Also, is this really P1? I think OS X, with the
> window bordering on the edge of the screen, is the only situation where the
> labels get cut off. At least on Windows, they looked fine when I checked
> yesterday... I was under the impression the labels had always been there.
This is locale-dependent anyway. Even if they were visible on all OSes for en-US, that wouldn't really mean anything for other locales.
Reporter | ||
Comment 3•11 years ago
|
||
Per the specs (see the live demo available at the URL), there is no label for the +/- button visible at all times, independent of the locale. Also in the older panel UI mockups, there were no labels to be seen there.
The labels were never supposed to be there and weren't there in the initial implementation (bug 870897). Somehow they got introduced and I therefore marked it a regression.
Updated•11 years ago
|
Keywords: regressionwindow-wanted
Assignee | ||
Comment 4•11 years ago
|
||
Good: http://hg.mozilla.org/projects/ux/rev/34c0f40827de (July 9th Mac nightly)
Bad: http://hg.mozilla.org/projects/ux/rev/727132bea25c (July 11th Windows nightly)
(no nightly on the 10th that I can see)
https://hg.mozilla.org/projects/ux/pushloghtml?tochange=727132bea25c&fromchange=34c0f40827de
Will try to narrow this down more. Still unsure this really needs to be P1.
Keywords: regressionwindow-wanted
Assignee | ||
Comment 5•11 years ago
|
||
Locally backed out https://hg.mozilla.org/projects/ux/rev/a39e19167f5e (bug 877684), which fixes this. Still looking where I went wrong, but taking anyway.
Reporter | ||
Comment 6•11 years ago
|
||
Gijs, thanks for narrowing that down! I was able to indeed pinpoint https://hg.mozilla.org/projects/ux/rev/a39e19167f5e - bug 877684 as the offending changeset.
More specifically, the changes in browser/themes/osx/customizableui/panelUIOverlay.css - https://hg.mozilla.org/projects/ux/rev/a39e19167f5e#l5.19
Assignee | ||
Comment 7•11 years ago
|
||
I was looking at this, but -moz-box is the default, so I don't really understand why we need to add this here. Getting rid of it doesn't seem to break anything on Windows, at least... Mike?
PS: I also think that these buttons should have an aria-label instead of a label if we're not actually wanting to ever show the text, but that's a separate bug I guess... because of our setAttributes magic that's not as obvious to fix.
Attachment #775634 -
Flags: review?(mconley)
Comment 8•11 years ago
|
||
Comment on attachment 775634 [details] [diff] [review]
Patch v1
Review of attachment 775634 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/themes/shared/customizableui/panelUIOverlay.inc.css
@@ +50,5 @@
>
> #PanelUI-contents > toolbarpaletteitem > toolbaritem > toolbarbutton > .toolbarbutton-text,
> #PanelUI-contents > toolbaritem > toolbarbutton > .toolbarbutton-text,
> #PanelUI-contents > toolbarpaletteitem > toolbarbutton > .toolbarbutton-text,
> #PanelUI-contents > toolbarbutton > .toolbarbutton-text {
Yeah, that -moz-box didn't seem necessary.
Attachment #775634 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 9•11 years ago
|
||
Whiteboard: [Australis:M?][Australis:P1] → [Australis:M8][Australis:P1][fixed-in-ux]
Assignee | ||
Comment 10•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M8][Australis:P1][fixed-in-ux] → [Australis:M8][Australis:P1]
Target Milestone: --- → Firefox 28
You need to log in
before you can comment on or make changes to this bug.
Description
•