Closed Bug 1208715 Opened 6 years ago Closed 6 years ago

Impose size restriction on extension icons

Categories

(Firefox :: Theme, defect)

44 Branch
defect
Not set
minor

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)

Attached image ff.PNG
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.
Severity: normal → minor
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
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
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)
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)
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
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)
So what do you think, Gijs? Should we add the restriction for toolbarbutton-1?
Flags: needinfo?(gijskruitbosch+bugs)
(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)
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)
I can take this.
Assignee: nobody → dao
Flags: needinfo?(gijskruitbosch+bugs)
Keywords: addon-compat
Thanks Dão!
Attached patch patch (obsolete) — Splinter Review
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)
Attached patch patchSplinter Review
rebased
Attachment #8670953 - Attachment is obsolete: true
Attachment #8670953 - Flags: review?(jaws)
Attachment #8671806 - Flags: review?(jaws)
Attachment #8671806 - Flags: review?(jaws) → review+
Summary: Impose size restriction on extension icons from browser base/theme code → Impose size restriction on extension icons
https://hg.mozilla.org/mozilla-central/rev/4d1bdb825e4c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
this patch mess-up all toolbarbutton-1 with type=menu
Depends on: 1214555
Depends on: 1214579
Attached image 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?
(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.
(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.
(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?)).
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.
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; }
Blocks: 1215100
Depends on: 1222747
(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.
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
Status: RESOLVED → VERIFIED
Is there a way to turn this off?
(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.
Any suggestions?
(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.
Any reason why users and add-on writers weren't warned of a change which would obviously affect them?
Keywords: dev-doc-needed
Duplicate of this bug: 768506
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)
It's mentioned in the blog post.
Flags: needinfo?(jorge)
You need to log in before you can comment on or make changes to this bug.