Closed Bug 1302088 Opened 8 years ago Closed 8 years ago

Regression on contextual identity menu icons are missing

Categories

(Firefox :: Theme, defect, P2)

50 Branch
x86_64
Linux
defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: jkt, Assigned: jkt)

References

(Blocks 1 open bug)

Details

(Whiteboard: [userContextId])

Attachments

(1 file)

STR:
1. Click File / alt+f
2. Click New Container Tab
3. Notice icons are missing

Regression appears to come from: https://bugzilla.mozilla.org/show_bug.cgi?id=589146
Assignee: nobody → jkt
Comment on attachment 8790259 [details]
Bug 1302088 - Fixing contextual identity menu icon to be visible by using the favicon class

This appears to be working as intended, i.e. we're respecting the icons-in-menus setting.

These icons aren't favicons, so we shouldn't use the menuitem-with-favicon class.
Attachment #8790259 - Flags: review?(dao+bmo) → review-
These icons have always been shown even with that option off and was part of the expected design.

Sure enough they are not favicons however they are indicators to the type of tab they are opening much like a favicon is so fall into the same category somewhat.

I'm happy to open up another class so we can keep the categorisation separate however they should be being shown.
(In reply to Jonathan Kingston [:jkt] from comment #3)
> These icons have always been shown even with that option off and was part of
> the expected design.

If they were shown then that was by mistake. I'm not sure what changed here if anything at all (the bug you linked to is years old and seems unrelated), but in any case the current behavior is the right behavior.

> Sure enough they are not favicons however they are indicators to the type of
> tab they are opening much like a favicon is so fall into the same category
> somewhat.

They're indicators just like every other icon we'd use in menus. They are however redundant since they basically repeat the label, so we should respect the system setting here, just like we do elsewhere.

Favicons for bookmark and history items are an exception because they may convey additional information that's not covered by the label.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
Likely the wrong regression then however I had a bug raised before that I fixed to display these.
Adding in Tanvi for her thoughts on this.
Perhaps we could add a setting in the about:preferences#containers panel?
Flags: needinfo?(tanvi)
Turns out the Gtk setting we're respecting here is deprecated. I've filed bug 1302157.
Flags: needinfo?(tanvi)
This is working correctly under both macOS 10.11.6 and Win 10. We should keep the UX consistent and make sure this is working under Linux as well..

Regression Range:
=================

6:54.81 INFO: Last good revision: 074fc3a0ac1f4ac5166194275ea7de7b3eb03d17
6:54.81 INFO: First bad revision: 2374e3ecf070b71e4e6cecc877785be383785064
6:54.81 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=074fc3a0ac1f4ac5166194275ea7de7b3eb03d17&tochange=2374e3ecf070b71e4e6cecc877785be383785064

6:55.88 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1299480
(In reply to Kamil Jozwiak [:kjozwiak] from comment #7)
> This is working correctly under both macOS 10.11.6 and Win 10. We should
> keep the UX consistent and make sure this is working under Linux as well..

Mac and Windows don't even have the setting that we're respecting on Linux, hence the difference. Anyway, this is all moot now, see my previous comment.
No longer blocks: 589146
Is the problem that the icons are always missing on Linux, or that they are missing when a particular preference is flipped?
Images are also missing from the following:

* right click context menu -> Open Link in New Container Tab
* down arrow (when there's many tabs opened) -> New Container Tab
Jonathan, is there a reason the fix in bug#1302157 wasn't uplifted into fx51? It looks like fx51 is the only build that's currently not displaying icons under the file menu and the right click context menu while using Ubuntu.

Builds being used:

* fx53.0a1, buildid: 20161115030213, changeset: 5e7676832766 - PASSED
* icons appearing as expected

* fx52.0a2, buildid: 20161115004018, changeset: 401aa89aeb11 - PASSED
** icons appearing as expected

* fx51.0b1, buildid: 20161114042748, changeset: f455459b2ae5 - FAILED
** icons are NOT appearing under the file menu/right click context menu

* fx50.0, buildid: 20161104212021, changeset: dc617d65c9f0 - PASSED
** icons appearing as expected
Flags: needinfo?(jkt)
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Whiteboard: [userContextId]
I thought it was because it was considered a risk to uplift a removal from CSS and change of so many icons. We could ask I guess?
Flags: needinfo?(tanvi)
Flags: needinfo?(kjozwiak)
Flags: needinfo?(jkt)
Kamil, is this linux only?
Flags: needinfo?(tanvi)
Priority: -- → P2
(In reply to Tanvi Vyas - behind on bugzilla [:tanvi] from comment #13)
> Kamil, is this linux only?

Yup, this is only affecting linux when using fx51.
Flags: needinfo?(kjozwiak)
Okay, since this is only Linux Firefox 51, I'm going to reclose this won't fix.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: