1594513 - Icons for external applications as handlers for files show an oversized icon in the applications section of the preferences / options

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Mike Kaply [:mkaply]]

Attachment: adobe.JPG

If you go to prefs and look at the dropdown for PDF, the adobe icon is the right size, but once you select it, it's oversized (see screenshot). When you navigate away, it is sized correctlyh again

We're getting the icon from:

moz-icon:file:///C:/Program%20Files%20(x86)/Adobe/Acrobat%20Reader%20DC/Reader/AcroRd32.exe?size=16

which is correct, but somehow in this case, the image is growing to the height of the menuitem.

We should be setting the size of this icon explicitly via CSS.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Bug 1594513 - Explicitly set width and height for application images. r?Gijs

Comment 2

[:Gijs (he/him)]

Do we know when this regressed?

Comment 3

[Mike Kaply [:mkaply]]

It's always been this way as far as I can tell (I went back to 2017)

I was able to actually display the

moz-icon:file:///C:/Program%20Files%20(x86)/Adobe/Acrobat%20Reader%20DC/Reader/AcroRd32.exe?size=16

file in an older Firefox and it's coming back as 32x32 even though we're asking for 16.

requesting 32 comes back as 64.

At first I thought it was related to my high DPI screen, but even when I change to standard DPI, I still get the wrong size images from moz-icon.

Unfortunately the moz-icon URLs just crash the tab in current versions of Firefox so not as easy to debug.

I'll check the moz-icon handler.

(Size note - Linux added the 16x16 css a long time ago)

Comment 4

[Mike Kaply [:mkaply]]

https://searchfox.org/mozilla-central/source/browser/components/preferences/in-content/main.js#3021

Is why it is correct in the menu when it doesn't have focus

The reason it's correct when popped down is because menu.css has:

.menu-iconic-icon {
width: 16px;
height: 16px;
}

So every other place it's used, they explicitly set the width/height to 16.

But I agree, something is wrong with the icon.

Comment 5

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #3)

It's always been this way as far as I can tell (I went back to 2017)

I was able to actually display the

moz-icon:file:///C:/Program%20Files%20(x86)/Adobe/Acrobat%20Reader%20DC/Reader/AcroRd32.exe?size=16

file in an older Firefox and it's coming back as 32x32 even though we're asking for 16.

Does the file provide a 16x16 icon?

requesting 32 comes back as 64.

At first I thought it was related to my high DPI screen, but even when I change to standard DPI, I still get the wrong size images from moz-icon.

How did you change DPI? Might take a restart of the windows system.

Unfortunately the moz-icon URLs just crash the tab in current versions of Firefox so not as easy to debug.

Oh? Got crash links? I initially thought this might be my fault for security reasons, but at least on mac it doesn't crash.

I'll check the moz-icon handler.

(Size note - Linux added the 16x16 css a long time ago)

I expect this might be due to gtk widget icons being used.

(In reply to Mike Kaply [:mkaply] from comment #4)

The reason it's correct when popped down is because menu.css has:

.menu-iconic-icon {
width: 16px;
height: 16px;
}

So every other place it's used, they explicitly set the width/height to 16.

Hm, why isn't menu.css applying to the icon when the menu is not open?

But I agree, something is wrong with the icon.

I'm not sure... It's sort of an odd thing that this is getting us the "wrong" size, but I don't want you to have to spend 2 days figuring out the 'why' here if all we need is a simple CSS fix... That said, this might be breaking in other places (e.g. the unknowncontenttype ("what do you want to do with this file") dialog)

Comment 6

[Mike Kaply [:mkaply]]

(In reply to :Gijs (he/him) from comment #5)

Does the file provide a 16x16 icon?

It does. I actually checked the Firefox icon to verify that.

requesting 32 comes back as 64.

And note that we don't have a 64x64 so we're generating that.

How did you change DPI? Might take a restart of the windows system.

I'll try that.

Unfortunately the moz-icon URLs just crash the tab in current versions of Firefox so not as easy to debug.

Oh? Got crash links? I initially thought this might be my fault for security reasons, but at least on mac it doesn't crash.

https://crash-stats.mozilla.org/report/index/3769d046-cf9e-4d1b-9223-a2b4f0191106
https://crash-stats.mozilla.org/report/index/ab59de50-d47b-4910-920b-bd87f0191106

I'm going to run mozregression to see when it started.

Hm, why isn't menu.css applying to the icon when the menu is not open?

Good question. I'll check that.

But I agree, something is wrong with the icon.

I'm not sure... It's sort of an odd thing that this is getting us the "wrong" size, but I don't want you to have to spend 2 days figuring out the 'why' here if all we need is a simple CSS fix... That said, this might be breaking in other places (e.g. the unknowncontenttype ("what do you want to do with this file") dialog)

I think it's worth spending at least a little time on. I'll keep looking.

Comment 7

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #6)

(In reply to :Gijs (he/him) from comment #5)

Unfortunately the moz-icon URLs just crash the tab in current versions of Firefox so not as easy to debug.

Oh? Got crash links? I initially thought this might be my fault for security reasons, but at least on mac it doesn't crash.

https://crash-stats.mozilla.org/report/index/3769d046-cf9e-4d1b-9223-a2b4f0191106
https://crash-stats.mozilla.org/report/index/ab59de50-d47b-4910-920b-bd87f0191106

I'm going to run mozregression to see when it started.

crash-stats (bugzilla tab) links to https://bugzilla.mozilla.org/show_bug.cgi?id=1583044 which suggests this is fixed on beta/nightly as of about 2 weeks ago. You're crashing on 70.0.1 so that seems right - current nightly or beta should work.

Comment 9

[:Gijs (he/him)]

This also affects mac, per the dupe.

Comment 10

[Mike Kaply [:mkaply]]

It's definitely related to DPI on Windows (and probably Mac)

When I set my windows to 1x DPI and rebotted, moz-icon:file:///C:/Program%20Files%20(x86)/Adobe/Acrobat%20Reader%20DC/Reader/AcroRd32.exe?size=16 returned a 16x16 icon.

I think this means that adding width/height is the correct fix because We do want to use the higher res icon when we can.

Comment 11

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #10)

It's definitely related to DPI on Windows (and probably Mac)

When I set my windows to 1x DPI and rebotted, moz-icon:file:///C:/Program%20Files%20(x86)/Adobe/Acrobat%20Reader%20DC/Reader/AcroRd32.exe?size=16 returned a 16x16 icon.

I think this means that adding width/height is the correct fix because We do want to use the higher res icon when we can.

Concur, thanks for digging!

Comment 12

[Mike Kaply [:mkaply]]

You asked one other question that I can answer:

Hm, why isn't menu.css applying to the icon when the menu is not open?

menu.css is specifically applied to the menu-iconic-icon (which is shared between popup menus and menuitems)

.menu-iconic-icon {
width: 16px;
height: 16px;
}

When the menulist is closed and selected, the image is a menulist-icon which gets no implicit sizing.

<image xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" class="menulist-icon" role="none" src="moz-icon://file:///C:/Program%20Files%20(x86)/Adobe/Acrobat%20Reader%20DC/Reader/AcroRd32.exe?size=16"/>

And the reason it's different between focused and unfocused is that when focused, it's a menulist, when unfocused it's:

<image xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" class="actionIcon" width="16" height="16" src="moz-icon://file:///C:/Program%20Files%20(x86)/Adobe/Acrobat%20Reader%20DC/Reader/AcroRd32.exe?size=16"/>

which explicitly sets the width/height.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0ceb4de01fb
Explicitly set width and height for application images. r=Gijs

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e0ceb4de01fb

Comment 15

[Cristian Fogel [:cfogel]]

Is there anything extra to this as far as STR/setup goes?
With 72.0a1(20191107215315) set the PDF action to the same Acrobat Reader but the icon sizes appear to be in order.

Comment 16

[:Gijs (he/him)]

(In reply to Cristian Fogel, QA [:cfogel] from comment #15)

Is there anything extra to this as far as STR/setup goes?
With 72.0a1(20191107215315) set the PDF action to the same Acrobat Reader but the icon sizes appear to be in order.

Are you testing on a hidpi machine? See comment #10.

Comment 17

[Mike Kaply [:mkaply]]

Yes, you have to have windows set to 150% or 200% dpi

Comment 18

[Cristian Fogel [:cfogel]]

Ah, that one totally slipped.
Will look into it as soon as our machine becomes available.

Thank you!

Comment 19

[Cristian Fogel [:cfogel]]

Confirmed the issue with a Nightly build from 06-11-2019.
Fix verified with 72.0.2.