The new icons are pretty... PRETTY ENORMOUS amirite!

RESOLVED FIXED in Firefox 28

Status

()

Firefox
Theme
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Dolske, Assigned: Gijs)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 28
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M6])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 753058 [details]
HUEG SCREENSHOT

Screenshot attached. The New Window and Print icons are the correct size.

I suspect this rule, added by bug 866863, just needs to set the default icon size (or a similar rule outside the hidpi block, since the dimensions are the same either way)...

browser/themes/osx/customizableui/panelUIOverlay.css

+@media (min-resolution: 2dppx) {
+  #PanelUI-contents[type="grid"] toolbarbutton,
+  #customization-palette toolbarbutton {
+    list-style-image: url(chrome://browser/skin/customizableui/menuPanel-icons@2x.png);
+  }
+
+  #PanelUI-popup .panel-combined-button {
+    list-style-image: url(chrome://browser/skin/customizableui/menuPanel-small-icons@2x.png);
+  }
Look... it was said that we should have a touch friendly UI. You got a touch friendly UI. What more could we give you! Gosh! 

:-P
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Created attachment 753220 [details] [diff] [review]
Stopgap Patch

I am unclear on why we're using something else than Toolbar.png... we now have buttons using Toolbar.png and using the new sprites, which leads to all kinds of craziness. For instance, if the bookmarks and history buttons are in the customization palette they have no icon at all... Are we waiting until we have final assets and small copies of all the icons in order to do that?

This patch fixes the hugeness, and uses the @x2 images for the combined buttons in customization mode, too (right now you can see them visually degrading if you go into customization mode...).

Ideally, I believe we want to merge all the new buttons into the primaryToolbarButton define in shared/browser.inc, but for that to work properly they need to share the same sprite, I suspect - I tried doing it to fix this bug and that led to icons disappearing for buttons in the customization palette. :-\
Attachment #753220 - Flags: review?(mconley)
Whiteboard: [Australis:M5] → [Australis:M6]
(In reply to :Gijs Kruitbosch from comment #2)
> Created attachment 753220 [details] [diff] [review]
> Stopgap Patch
> 
> I am unclear on why we're using something else than Toolbar.png... we now
> have buttons using Toolbar.png and using the new sprites, which leads to all
> kinds of craziness. For instance, if the bookmarks and history buttons are
> in the customization palette they have no icon at all... Are we waiting
> until we have final assets and small copies of all the icons in order to do
> that?

As Stephen mentioned earlier, the reason we're splitting out from Toolbar.png is because of the higher maintenance cost of having icons with different dimensions in the same spritesheet.

So there's no plans at this point to toss them into Toolbar.png.
Comment on attachment 753220 [details] [diff] [review]
Stopgap Patch

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

The code looks fine, but I don't have a Retina Macbook to test this with. I'll spin up a build and see if somebody is willing to be my guinea pig.
Comment on attachment 753220 [details] [diff] [review]
Stopgap Patch

According to some Retina-using folks, this patch makes everything jake.

Since Gijs isn't going to be here until Tuesday, I'll land this for him.
Attachment #753220 - Flags: review?(mconley) → review+
https://hg.mozilla.org/mozilla-central/rev/a77f00615188
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M6][fixed-in-ux] → [Australis:M6]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.