Even though Screenshots is becoming a webExtenison, it should appear in the pageAction as though it is a native Firefox tool, since users do not install it themselves and cannot uninstall it. We need to make sure of the following: - in the meatball menu, Screenshots is placed in alphabetical order among other native tools such as `Copy Link` and `Share` rather than being placed with other user-installed webExtensions icons - When users context click on the Screenshots pageAction, the context click options should be `Remove from Address Bar` or `Add to Address Bar` only, and not the webExtensions options of `Don't show in address bar` and `Manage Extension`
- How much of this should the addons team own, vs the screenshots team? This mostly seems like core addons stuff to me, since these same issues will recur with future system addons. - Is this doable in the 63 timeframe? If not, I think we'll want to revert bug 1466575 until 63 branches.
Per conversations in IRC, moving this over to the WebExtensions team. We will need someone to verify that all requested changes are possible, and in what timeframe.
Component: Screenshots → Frontend
Flags: needinfo?(aswan) → needinfo?(ddurst)
Priority: -- → P2
Product: Firefox → WebExtensions
Note that the webextension page action is now disabled on about:reader pages, but Screenshots is supposed to work in reader mode. (Do I need a separate bug for this?)
8 months ago
Depends on: 1483620
I'm going to proactively NI adw or gijs to speak to "is this doable in the 63 timeframe?" question, as the implementation is largely in frontend.
No longer depends on: 1483620
I will take a look at this.
Assignee: nobody → mstriemer
I'm not sure what all the consequences are of making it a real webextension -- the webextensions team probably knows better than me when it comes to things like permissions (like the fact that the action is now disabled on about:reader is news to me). But I can comment on some of it: (In reply to [:jgruen] from comment #0) > - in the meatball menu, Screenshots is placed in alphabetical order among > other native tools such as `Copy Link` and `Share` rather than being placed > with other user-installed webExtensions icons Bug 1483088 covers this, and I agree that it shouldn't be marked as a dupe. All the work here is in webextensions land. Or, there's another way we could do this, which is to whitelist screenshots directly in PageActions.jsm, in which case that's on the front-end team (like me). But IMO that's not as good a fix. (BTW the ordering of the built-in actions isn't entirely alphabetical. It's more ad hoc.) > - When users context click on the Screenshots pageAction, the context click > options should be `Remove from Address Bar` or `Add to Address Bar` only, > and not the webExtensions options of `Don't show in address bar` and `Manage > Extension` There are two parts to the context menu. One part is choosing the labeling of the add/remove menu items -- we say "add" and "remove" for built-in actions and "show" and "don't show" for extensions. Barry's patch in bug 1483085 will fix that. The other part is not adding the "Manage Extension" menu item. That item is added by webextensions code.
Maybe one of the devs here can also take a look at this issue, which is probably related: Bug 1482697 Thanks.
[Tracking Requested - why for this release]: Firefox Screenshots is migrating to a webextension in the 64 release. This is one of two UI bugs related to this change that we'd like to fix for the 64 release.
Here's a simple patch to treat it as a built-in action. It was looking through the gBuiltInActions array before, but there's code to check that along with our built-in add-on ids on the action that makes it work.
Here's a screenshot of where the page action will go after this patch. Is that a good spot?
Comment on attachment 9010446 [details] Bug 1483591 - Treat built-in webextension page actions as built-in r?adw Drew Willcoxon :adw has approved the revision.
Attachment #9010446 - Flags: review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/13b93b43ad5e Treat built-in webextension page actions as built-in r=adw
Moving this bug to Firefox since it ended up being about the browser front end and not webextensions
Component: Frontend → Toolbars and Customization
Product: WebExtensions → Firefox
Target Milestone: mozilla64 → ---
I have reproduced this bug with Nightly 63.0a1 (2018-08-15) on Windows 7, 64 Bit! This bug's fix is verified with latest Beta! Build ID 20181122182000 User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0
QA Whiteboard: [testday-20181123]
Thanks Mohammad. Marking this accordingly.
You need to log in before you can comment on or make changes to this bug.