Missing page actions menu icons

RESOLVED FIXED in Firefox 61

Status

()

defect
RESOLVED FIXED
Last year
Last year

People

(Reporter: maurosan, Assigned: kmag)

Tracking

61 Branch
Firefox 61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox59 unaffected, firefox60 unaffected, firefox61+ fixed)

Details

Attachments

(1 attachment)

Reporter

Description

Last year
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180327105613

Steps to reproduce:

* This is seen either with some webextensions, in safe mode or even in a brand new profile since a few Nightlies back.
* I believe this was OK at the start of the 61 cycle.
* Click on the page actions menu (ellipsis icon in Address Bar).


Actual results:

* While the "Take a Screenshot" and "Report Site Issue..." are available and functional their icons are missing.
* Adding their icons to the Address Bar with the context menu does not change this.
* No other page action is affected.


Expected results:

* The "Take a Screenshot" and "Report Site Issue..." page actions should display their respective icons.
Reporter

Updated

Last year
Severity: normal → minor
Component: Untriaged → Address Bar
OS: Unspecified → Windows 10
Hardware: Unspecified → x86
Reporter

Updated

Last year
Hardware: x86 → x86_64
Drew, any idea?
Flags: needinfo?(adw)
Interesting, they're the only two actions that are provided entirely through extensions.  The only thing that changed with page actions recently is bug 1446250, and it did touch icons.  Kris, could you take a look please?

I can't reproduce the problem though.
Flags: needinfo?(adw) → needinfo?(kmaglione+bmo)
See Also: → 1449947
Other people are reporting this same problem for the new "add search engine" action, so I'm going to mark this as confirmed even though I haven't confirmed it myself.  I can't reproduce the problem on macOS, so I'm wondering if this only happens on Windows and Linux, but that seems weird.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Drew Willcoxon :adw from comment #2)
> Interesting, they're the only two actions that are provided entirely through
> extensions.

The fact that people are seeing this with the new "add search engine" action points to this being a problem with all the actions that aren't in browser.xul or don't otherwise add DOM nodes themselves -- i.e., everything but the bookmark star and Pocket.
Er, maybe not.  The other built-in actions specify their icons in CSS.  They don't call action.setIconURL().  The "add search engine" action does call setIconURL().  So maybe it has something to do with that.  Anyway, this needs investigation.
It's something to do with screen DPI.  I'm on a "retina" screen.  No problem.  I set layout.css.devPixelsPerPx=1, problem.
Reporter

Comment 7

Last year
Thanks for tackling this issue; this seems to be in the right track: doing the converse (Fx x64 on Windows 10 x64 laptop, standard panel display) by setting layout.css.devPixelsPerPx=2 from its -1 default makes the icons appear (it's no workaround, of course).
Reporter

Comment 8

Last year
(Sorry for the spam) On the other hand, just noticed the new page action for adding a new search engine and its icon renders properly (at least for Bugzilla and YouTube) in my default conditions (layout.css.devPixelsPerPx=-1).
It probably has to do with the size of the icon, not only your DPI.

I suspect the problem is here: https://dxr.mozilla.org/mozilla-central/rev/6aa3b57955fed5e137d0306478e1a4b424a6d392/browser/modules/PageActions.jsm#736
Blocks: 1446250
Isn't this just a straight dupe of bug 1447943? Though perhaps at this point this bug has more info...
Well, that bug is filed in webextensions...  But yeah, the cause is this bug.
Blocks: 1447943
Also, bug 1446250 removed all callers of PageActions.iconURLForSize (the non-underscored one) but not the method itself, so the fix for this bug might as well do that too.
Severity: minor → normal
OS: Windows 10 → All
Hardware: x86_64 → All
Comment hidden (mozreview-request)
Assignee: nobody → kmaglione+bmo
No longer blocks: 1447943
Duplicate of this bug: 1447943

Comment 15

Last year
mozreview-review
Comment on attachment 8963824 [details]
Bug 1449200: Return icon URL for all sizes when processing an icon string.

https://reviewboard.mozilla.org/r/232694/#review238174

r=me with Drew's feedback incorporated.

::: browser/modules/PageActions.jsm:819
(Diff revision 1)
>     *
>     * @param  peferredSize (number, required)
>     *         The icon size you prefer.
>     * @return The URL of the best icon, or null.
>     */
>    iconURLForSize(preferredSize, browserWindow) {

As Drew noted:

(In reply to Drew Willcoxon :adw from comment #12)
> Also, bug 1446250 removed all callers of PageActions.iconURLForSize (the
> non-underscored one) but not the method itself, so the fix for this bug
> might as well do that too.



please rm this and move the comment to the underscored method, removing the "@see ..." clause there.
Attachment #8963824 - Flags: review?(gijskruitbosch+bugs) → review+
Assignee

Comment 16

Last year
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9052a77c9906a9be02b8442c10346c20fcd486d
Bug 1449200: Return icon URL for all sizes when processing an icon string. r=Gijs

Comment 17

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/e9052a77c990
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Flags: needinfo?(kmaglione+bmo)
You need to log in before you can comment on or make changes to this bug.