Impose size restriction on extension icons

VERIFIED FIXED in Firefox 44

Status

()

Firefox
Theme
--
minor
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: faisalid, Assigned: dao)

Tracking

({addon-compat, dev-doc-needed, regression})

44 Branch
Firefox 44
addon-compat, dev-doc-needed, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 verified)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Created attachment 8666329 [details]
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.
(Reporter)

Updated

3 years ago
Severity: normal → minor

Comment 1

3 years ago
What icon is that? It looks like an add-on icon; the issue should be reported to the author.
Flags: needinfo?(faisalid)
(Reporter)

Comment 2

3 years ago
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)

Updated

3 years ago
Summary: large icon next to address bar → large add-on icon next to address bar

Comment 3

3 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
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
(Assignee)

Comment 7

3 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)
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)
(Assignee)

Comment 11

3 years ago
I can take this.
Assignee: nobody → dao
Flags: needinfo?(gijskruitbosch+bugs)
Keywords: addon-compat
Thanks Dão!
(Assignee)

Comment 13

3 years ago
Created attachment 8670953 [details] [diff] [review]
patch

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

3 years ago
Created attachment 8671806 [details] [diff] [review]
patch

rebased
Attachment #8670953 - Attachment is obsolete: true
Attachment #8670953 - Flags: review?(jaws)
Attachment #8671806 - Flags: review?(jaws)
Attachment #8671806 - Flags: review?(jaws) → review+
(Assignee)

Updated

3 years ago
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
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44

Comment 17

3 years ago
this patch mess-up all toolbarbutton-1 with type=menu

Comment 18

3 years ago
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?
(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

3 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.
(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

3 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

3 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; }
Depends on: 1222747

Comment 24

3 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

3 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
Status: RESOLVED → VERIFIED
status-firefox44: fixed → verified

Comment 26

2 years ago
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.

Comment 28

2 years ago
Any suggestions?

Comment 29

2 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

2 years ago
Any reason why users and add-on writers weren't warned of a change which would obviously affect them?

Updated

2 years ago
Keywords: dev-doc-needed
Duplicate of this bug: 768506

Comment 32

2 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)
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.