Closed
Bug 1208715
Opened 9 years ago
Closed 9 years ago
Impose size restriction on extension icons
Categories
(Firefox :: Theme, defect)
Tracking
()
VERIFIED
FIXED
Firefox 44
Tracking | Status | |
---|---|---|
firefox44 | --- | verified |
People
(Reporter: faisalid, Assigned: dao)
References
Details
(Keywords: addon-compat, dev-doc-needed, regression)
Attachments
(3 files, 1 obsolete file)
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0 Build ID: 20150925030206 Steps to reproduce: Update firefox nightly Expected results: icon should not have exceeded the maximum size.
Comment 1•9 years ago
|
||
What icon is that? It looks like an add-on icon; the issue should be reported to the author.
Flags: needinfo?(faisalid)
Yes, you are right it is an addon icon for Integrated Google Calendar. I was under the impression there is a default icon size limit which addons aren't allowed to exceed. And the problem appeared after firefox was updated so I'm guessing the icon size limit was removed hence the enlarged icon.
Flags: needinfo?(faisalid)
Summary: large icon next to address bar → large add-on icon next to address bar
Comment 3•9 years ago
|
||
regressionwindow |
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:43.0) Gecko/20100101 Firefox/43.0 20150920030217 Push log: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4313752f69956ae248bd4e7ff3913c8dd4252698&tochange=ccd6b5f5e544c1d708849144943a776941bd3794 Suspect: Bug 1201796 The add-on is using a remote icon, e.g. https://calendar.google.com/googlecalendar/images/favicon_v2014_27.ico The remote icon includes a 16×16 image, plus a 32×32 image. Prior to build 20150920030217, Firefox would pick the smaller size.
Blocks: 1201796
Status: UNCONFIRMED → NEW
Component: Untriaged → ImageLib
Ever confirmed: true
Flags: needinfo?(seth)
Keywords: regression
Product: Firefox → Core
Comment 4•9 years ago
|
||
Yes, the largest resource in an icon now determines its intrinsic size. To fix this kind of problem, normally a content author should specify an explicit width and height for the icon using CSS. (16x16 CSS pixels in this case.) However, because this is an addon icon, presumably the icon is specified in the addon's manifest, so there's no way to apply CSS to it. I don't see an easy fix at the platform level here, unfortunately, either. Obviously this problem could be solved just by the addon author including their own 16x16 icon. But I'm a little surprised that's necessary; shouldn't we be specifying a size for these icons in the frontend code? I'm investigating...
Flags: needinfo?(seth)
Comment 5•9 years ago
|
||
Gijs, looks like this needs a fix on the frontend side. You mentioned on IRC that we might need to set the size of toolbarbutton-1 icons to 16x16.
Flags: needinfo?(gijskruitbosch+bugs)
Comment 6•9 years ago
|
||
Dão, can you comment? I would imagine we already have a bug on file about this if we actually thought it was a good idea, and I also vaguely recall we tried to do something like this for Australis and had to abandon it, but maybe I'm wrong?
Component: ImageLib → Theme
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(dao)
Product: Core → Firefox
Summary: large add-on icon next to address bar → Impose size restriction on extension icons from browser base/theme code
Assignee | ||
Comment 7•9 years ago
|
||
A few vocal add-on authors complained about the size restriction, so we removed it. Then SDK people came along and requested that we add the restriction for SDK-provided add-ons, so we did that. I wouldn't be opposed to bringing back the restriction back for all toolbarbutton-1 icons.
Flags: needinfo?(dao)
Comment 8•9 years ago
|
||
So what do you think, Gijs? Should we add the restriction for toolbarbutton-1?
Flags: needinfo?(gijskruitbosch+bugs)
Comment 9•9 years ago
|
||
(In reply to Seth Fowler [:seth] [:s2h] from comment #8) > So what do you think, Gijs? Should we add the restriction for > toolbarbutton-1? I think we can try it. I don't know that it should be really high priority, and I don't know if it'll be easier to do it this time than it was last time...
Flags: needinfo?(gijskruitbosch+bugs)
Comment 10•9 years ago
|
||
Can we assign this to someone on the frontend team? I don't really have the knowledge to make this change myself, unfortunately...
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Comment 11•9 years ago
|
||
I can take this.
Comment 12•9 years ago
|
||
Thanks Dão!
Assignee | ||
Comment 13•9 years ago
|
||
I'm using max-width instead of width so that we don't upscale small icons, only downscale large ones.
Attachment #8670953 -
Flags: review?(jaws)
Assignee | ||
Comment 14•9 years ago
|
||
rebased
Attachment #8670953 -
Attachment is obsolete: true
Attachment #8670953 -
Flags: review?(jaws)
Attachment #8671806 -
Flags: review?(jaws)
Updated•9 years ago
|
Attachment #8671806 -
Flags: review?(jaws) → review+
Assignee | ||
Updated•9 years ago
|
Summary: Impose size restriction on extension icons from browser base/theme code → Impose size restriction on extension icons
Comment 16•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4d1bdb825e4c
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Comment 17•9 years ago
|
||
this patch mess-up all toolbarbutton-1 with type=menu
Comment 18•9 years ago
|
||
Is this patch the cause of the small icons on the sidebar title bar that is a feature of the add-on OmniSidebar?
Comment 19•9 years ago
|
||
(In reply to Gary [:streetwolf] from comment #18) > Created attachment 8673910 [details] > osb.png > > Is this patch the cause of the small icons on the sidebar title bar that is > a feature of the add-on OmniSidebar? This screenshot is so small I can't work out what it is showing.
Comment 20•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #19) > (In reply to Gary [:streetwolf] from comment #18) > > Created attachment 8673910 [details] > > osb.png > > > > Is this patch the cause of the small icons on the sidebar title bar that is > > a feature of the add-on OmniSidebar? > > This screenshot is so small I can't work out what it is showing. The first icon is the Sidebar Bookmarks icon and the second one is the Sidebar History icon. All of the icons on the customize screen are the proper size. It's just when I move them to the titlebar via OSB that they get microscopic.
Comment 21•9 years ago
|
||
(In reply to Gary [:streetwolf] from comment #20) > (In reply to :Gijs Kruitbosch from comment #19) > > (In reply to Gary [:streetwolf] from comment #18) > > > Created attachment 8673910 [details] > > > osb.png > > > > > > Is this patch the cause of the small icons on the sidebar title bar that is > > > a feature of the add-on OmniSidebar? > > > > This screenshot is so small I can't work out what it is showing. > > The first icon is the Sidebar Bookmarks icon and the second one is the > Sidebar History icon. All of the icons on the customize screen are the > proper size. It's just when I move them to the titlebar via OSB that they > get microscopic. You mean the tabstrip which is in the titlebar? Why do you need OmniSidebar (is this what OSB means?) to move the icons? As I said elsewhere, please file a new bug with more detail (esp. more context in the screenshot, and more detailed steps to reproduce, and things like platform info (are you on Linux? Windows? Something else?)).
Comment 22•9 years ago
|
||
You can't move icons onto the sidebars titlebar unless you use an add-on. OmniSidebar (OSB) allows me to do this. I'm on Windows 10 x64 and on Fx44 inbound x64-PGO. Normally I'd run this by the developer whom I have a good working relationship with but I suspect he is not available currently.
Comment 23•9 years ago
|
||
I was able to fix the icons with this snippet of css code: .toolbarbutton-1 > .toolbarbutton-icon, .toolbarbutton-1 > :-moz-any(.toolbarbutton-menubutton-button, .toolbarbutton-badge-stack) > .toolbarbutton-icon { max-width: 48px !important; }
Comment 24•9 years ago
|
||
(In reply to Gary [:streetwolf] from comment #23) > I was able to fix the icons with this snippet of css code: > > .toolbarbutton-1 > .toolbarbutton-icon, > .toolbarbutton-1 > :-moz-any(.toolbarbutton-menubutton-button, > .toolbarbutton-badge-stack) > .toolbarbutton-icon { max-width: 48px > !important; } I had to reduce the number to 28px. By other side, I'm using the beta version (43.0b3) and this isn't resolved.
Comment 25•8 years ago
|
||
I have successfully reproduce this bug on firefox nightly 44.0a1 (2015-09-26) with windows 8 (32 bit) Build ID :20150926030216 Mozilla/5.0 (Windows NT 6.3; rv:44.0) Gecko/20100101 Firefox/44.0 I found this fix on latest aurora 44.0a2 (2015-12-03 Mozilla/5.0 (Windows NT 6.3; rv:44.0) Gecko/20100101 Firefox/44.0 Build ID :20151203004003
Updated•8 years ago
|
Status: RESOLVED → VERIFIED
Comment 26•8 years ago
|
||
Is there a way to turn this off?
Comment 27•8 years ago
|
||
(In reply to dlande from comment #26) > Is there a way to turn this off? Add-ons can override the CSS if they wish to, yes.
Comment 28•8 years ago
|
||
Any suggestions?
Comment 29•8 years ago
|
||
(In reply to dlande from comment #28) > Any suggestions? Nevermind, the lovely people at Classic Theme Restorer are on it. Although, they appear to be a bit behind the curve, indicating that the communication effort in this regard was less than it should have been.
Comment 30•8 years ago
|
||
Any reason why users and add-on writers weren't warned of a change which would obviously affect them?
Keywords: dev-doc-needed
Comment 32•8 years ago
|
||
Jorge, could you add this bug to the add-on dev doc when you have some time, please. https://blog.mozilla.org/addons/2015/12/29/compatibility-for-firefox-44/ + https://developer.mozilla.org/en-US/Firefox/Releases/44
Flags: needinfo?(jorge)
You need to log in
before you can comment on or make changes to this bug.
Description
•