Closed
Bug 1302088
Opened 8 years ago
Closed 8 years ago
Regression on contextual identity menu icons are missing
Categories
(Firefox :: Theme, defect, P2)
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 | ||
Updated•8 years ago
|
Assignee: nobody → jkt
Comment hidden (mozreview-request) |
Comment 2•8 years ago
|
||
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-
Assignee | ||
Comment 3•8 years ago
|
||
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.
Comment 4•8 years ago
|
||
(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
Assignee | ||
Comment 5•8 years ago
|
||
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)
Comment 6•8 years ago
|
||
Turns out the Gtk setting we're respecting here is deprecated. I've filed bug 1302157.
Flags: needinfo?(tanvi)
Comment 7•8 years ago
|
||
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
Comment 8•8 years ago
|
||
(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.
Comment 9•8 years ago
|
||
Is the problem that the icons are always missing on Linux, or that they are missing when a particular preference is flipped?
Comment 10•8 years ago
|
||
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
Comment 11•8 years ago
|
||
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)
Updated•8 years ago
|
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Whiteboard: [userContextId]
Assignee | ||
Comment 12•8 years ago
|
||
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)
Updated•8 years ago
|
Priority: -- → P2
Comment 14•8 years ago
|
||
(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)
Comment 15•8 years ago
|
||
Okay, since this is only Linux Firefox 51, I'm going to reclose this won't fix.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•