Closed Bug 971948 Opened 10 years ago Closed 10 years ago

Bookmark toolbar items icon is broken in the palette

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 30
Tracking Status
firefox29 --- verified
firefox30 --- verified

People

(Reporter: u428464, Assigned: jaws)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P5])

Attachments

(2 files)

It just looks completely distorted.
Summary: Bookmark toolbar item's icon is broken in the palette → Bookmark toolbar items icon is broken in the palette
Whiteboard: [Australis:P4]
Most likely broken by bug 897496.
Blocks: 897496
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [Australis:P4] → [Australis:P5]
Attached image Screen Shot
OS: Windows 7 → All
This is resolved at least on Windows.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
This is still looking distorted for me on fx-team 363beb46bd10 Win8.1.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Status: REOPENED → NEW
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)
> This is still looking distorted for me on fx-team 363beb46bd10 Win8.1.

Same on a new profile on Win 7. Seems to be fine on OS X 10.9.
(In reply to :Gijs Kruitbosch from comment #5)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)
> > This is still looking distorted for me on fx-team 363beb46bd10 Win8.1.
> 
> Same on a new profile on Win 7. Seems to be fine on OS X 10.9.

Huh, and of course straight after that I can't reproduce on Win 7 anymore. Something weird is up here. :-\
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
The stretching of the icon is no longer present, I think that got fixed recently by removing the overflow:hidden, etc.

This patch fixes the positioning of the label in the palette and menu panel.
Attachment #8390801 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8390801 [details] [diff] [review]
Patch

Review of attachment 8390801 [details] [diff] [review]:
-----------------------------------------------------------------

r=me on the min-height, but I'm confused about the label's margin...

::: browser/themes/shared/customizableui/panelUIOverlay.inc.css
@@ +133,5 @@
>  
>  .panelUI-grid .toolbarbutton-1 > .toolbarbutton-text,
>  .panelUI-grid .toolbarbutton-1 > .toolbarbutton-multiline-text {
>    text-align: center;
> +  /* Need to override toolkit theming which sets margin: 0 !important; */

On what OS do we need to override this? I'm having to modify this for the subviewbuttons already (although they might not be toolbarbutton-1, come to think of it), and I'd like to not exacerbate our use of !important too much. :-(
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> We need 'important' for Linux and Windows, not OSX:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/linux/global/
> toolbarbutton.css#32
> http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/osx/global/
> toolbarbutton.css#24
> http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/
> toolbarbutton.css#30
> 
> Would you rather that I write the following?
> %ifdef XP_MACOSX
> margin: 2px 0 0;
> %else
> margin: 2px 0 0 !important;
> %endif

Nah, it's ok.
Attachment #8390801 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/integration/fx-team/rev/95341f53d78f
Whiteboard: [Australis:P5] → [Australis:P5][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/95341f53d78f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P5][fixed-in-fx-team] → [Australis:P5]
Target Milestone: --- → Firefox 30
Comment on attachment 8390801 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 897496
User impact if declined: label for palette item is out of line with other items
Testing completed (on m-c, etc.): locally and m-c
Risk to taking this patch (and alternatives if risky): none expected
String or IDL/UUID changes made by this patch: none
Attachment #8390801 - Flags: approval-mozilla-aurora?
Attachment #8390801 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: