Closed Bug 1370686 Opened 7 years ago Closed 7 years ago

Extensions sidebar icon doesn't show up in sidebar switcher panel

Categories

(Firefox :: General, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 57
Iteration:
57.2 - Aug 29
Tracking Status
firefox57 --- verified
firefox58 --- verified

People

(Reporter: mkaply, Assigned: bwinton)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [reserve-photon-structure] [sidebar])

Attachments

(2 files)

Currently if you add an icon to a sidebar, it's shown in the dropdown for the sidebar button (where there are no other icons). If you then open the sidebar and click the title, you'll see a dropdown where Firefox shows icons for Bookmarks/History/Synced Tabs, but the WebExtension sidebar does not have an icon. We should match Firefox in both cases.
Flags: needinfo?(amckay)
Whiteboard: [sidebar]
The toolbarbutton above will loose the dropdown, but the icon shouldn't be missing from the menu in the sidebar titlebar.
In my latest nightly, the button has no icon. The sidebar menu that photon adds has an icon. All looks consistent and nice to me.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(amckay)
Resolution: --- → WORKSFORME
Attached image sidebar_icons.png (deleted) —
I'm not seeing the sidebar icon anywhere at all with the example sidebar extension on latest Nightly. Could this please be reopened?
Flags: needinfo?(amckay)
Sure, it was there a couple of days ago, now it is gone. I'd guess this is related to all the photon changes.
Status: RESOLVED → REOPENED
Component: WebExtensions: General → General
Flags: needinfo?(amckay)
Product: Toolkit → Firefox
Resolution: WORKSFORME → ---
Summary: Sidebar icon is used in the opposite place as Firefox → Extensions sidebar icon doesn't show up
Can someone check if this is working in 55 beta, and, if not, set the relevant release tracking flags?
Blocks: 1353421, 1355324
Summary: Extensions sidebar icon doesn't show up → Extensions sidebar icon doesn't show up in sidebar switcher panel
Whiteboard: [sidebar] → [photon-structure][triage][sidebar]
Works in beta 55. Couldn't reproduce with or without the photon tracking flag.
I can't reproduce on OS X when loading the manifest.json for annotate-page from https://github.com/mdn/webextensions-examples (which I assume is what folks mean by 'example add-on'?) as a temporary add-on from about:debugging in today's nightly. Can someone provide more detailed STR? Is this Windows- or Linux-specific, or does it only happen after a restart, or ...?
Flags: needinfo?(kestrel)
It's due to the combination of these two styles on Windows 10, remove one or the other and the icon displays: #sidebarMenu-popup .subviewbutton-iconic > .toolbarbutton-icon { padding-inline-start: 16px; } toolbarbutton.webextension-menuitem > .toolbarbutton-icon { width: 16px; height: 16px; }
Flags: needinfo?(kestrel)
(In reply to Kestrel from comment #8) > It's due to the combination of these two styles on Windows 10, remove one or > the other and the icon displays: > > #sidebarMenu-popup .subviewbutton-iconic > .toolbarbutton-icon { > padding-inline-start: 16px; > } Ah, and this rule is %ifndef XP_MACOSX ( https://dxr.mozilla.org/mozilla-central/rev/bb8eab3c3ac4147848c4c85d628ba72029978665/browser/themes/shared/sidebar.inc.css#112-120 ), which explains why I couldn't reproduce on OS X...
Status: REOPENED → NEW
Flags: qe-verify?
Priority: -- → P2
Whiteboard: [photon-structure][triage][sidebar] → [photon-structure] [sidebar]
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly
Assignee: nobody → bwinton
Status: NEW → ASSIGNED
Priority: P2 → P1
For add-ons with icons, it looks like https://cl.ly/2C323N0s3S0q and for add-ons without icons, it looks like https://cl.ly/460S3h1C3k3e
This looks great! I would recommend using the default icon color (see Bookmarks/History/Synced Tabs) when we use the add-ons icon and not the custom icon.
Attachment #8887158 - Flags: review?(shorlander)
Comment on attachment 8887158 [details] Bug 1370686 - Show the WebExtensions icon in the sidebar dropdown. , ui-r=shorlander https://reviewboard.mozilla.org/r/157904/#review163076 ::: browser/components/extensions/ext-sidebarAction.js:204 (Diff revision 1) > + > + let sidebar = menuitem.ownerDocument.getElementById('sidebar-header'); > + sidebar.setAttribute("style", ` > + --webextension-menuitem-image: url("${getIcon(16)}"); > + --webextension-menuitem-image-2x: url("${getIcon(32)}"); > + `); This is wrong. I should be setting this when the panel changes, not when it's built. (But I can't seem to find the event that occurs when the panel is shown…) ::: browser/themes/shared/sidebar.inc.css:149 (Diff revision 1) > -moz-context-properties: fill; > fill: currentColor; > opacity: 0.8; > } > + > +#sidebar-box[sidebarcommand$="-sidebar-action"] > #sidebar-header > #sidebar-switcher-target > #sidebar-icon { We want a little more stuff here, to handle the case where we don't have an icon, and use the default one.
Comment on attachment 8887158 [details] Bug 1370686 - Show the WebExtensions icon in the sidebar dropdown. , ui-r=shorlander https://reviewboard.mozilla.org/r/157906/#review163404 Clearing review for now given the self-review. I expect you should be able to use a popupshowing listener on the popup (`#sidebarMenu-popup`) to attach when the popup is being shown rather than when it is built? ::: browser/base/content/browser.css (Diff revision 1) > list-style-image: var(--webextension-menuitem-image-2x, inherit); > } > } > > toolbarbutton.webextension-menuitem > .toolbarbutton-icon { > - width: 16px; Out of curiosity, why remove this? I rather expect it should be kept, so that webextensions that ship a 32x16px (or 16x32px) icon still end up with a 'square' slot for their icon.
Attachment #8887158 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8887158 [details] Bug 1370686 - Show the WebExtensions icon in the sidebar dropdown. , ui-r=shorlander https://reviewboard.mozilla.org/r/157906/#review163404 > Out of curiosity, why remove this? I rather expect it should be kept, so that webextensions that ship a 32x16px (or 16x32px) icon still end up with a 'square' slot for their icon. Without this, there's no space for the checkmark beside the icon. Perhaps there's a better way to do that, by adding a margin for the checkmark instead of padding? I'll look into it.
Attachment #8887158 - Flags: review?(shorlander) → review+
Blocks: 1387512
Whiteboard: [photon-structure] [sidebar] → [reserve-photon-structure] [sidebar]
On FF56.0b1 (Win 64), I also noticed icons are missing from the new Sidebar popup as well as Menu -> View -> Sidebar
Comment on attachment 8887158 [details] Bug 1370686 - Show the WebExtensions icon in the sidebar dropdown. , ui-r=shorlander https://reviewboard.mozilla.org/r/157906/#review174034 Fantastic, A++, would review again.
Attachment #8887158 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by bwinton@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b33b2839e981 Show the WebExtensions icon in the sidebar dropdown. r=Gijs, ui-r=shorlander
Status: ASSIGNED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Iteration: --- → 57.2 - Aug 29
Hey Gijs, can you clarify the STR for this?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Grover Wimberly IV [:Grover-QA] from comment #23) > Hey Gijs, can you clarify the STR for this? STR: 1. install https://addons.mozilla.org/en-US/firefox/addon/tree-style-tab/ (or another add-on that provides a sidebar) 2. open the sidebar 3. click the side bar header to get the dropdown with a list of sidebars you might want to open ER: the entry for tree style tabs has an icon AR (pre-patch): no icon on that entry Does that help?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(gwimberly)
Verified on Windows 10 64bit, Mac OS X 10.11, Ubuntu 16.04 64bit using Nightly 58.0a1 (64-bit) as of this date.
Flags: needinfo?(gwimberly)
Verified fixed on 57 Beta 6 (20171005195903) by following the STR from comment 24. Tested on Windows 10 x64, Mac OS X 10.11 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: