46 bytes, text/x-phabricator-request
|Details | Review|
Right now, built‑in page actions use “Remove from Address Bar” and “Add to Address Bar”, whereas WebExtension page actions use “Don’t Show in Address Bar” and “Show in Address Bar” respectively. Is there a reason why it’s different other than it just sort‑of happened and now there’s two separate strings for adding and removing page actions?
This seems like a bug, yes.
Sending this to UX for a decision on the final copy.
Priority: -- → P3
Let's match the built in strings.
It's not a bug, it was a deliberate decision by UX when this originally landed. Needinfo'ing Aaron and Bryan -- not saying we shouldn't do this, but they should be consulted at least
IIRC the reasoning for the difference is that the built-in actions are (generally) always shown in the urlbar, but extension actions are more likely to be shown only in certain contexts. So adding a built-in action adds it to the urlbar immediately, but adding an extension action only means that it's allowed to be shown in the urlbar when the extension so chooses. If extension actions used "Add", it might be the case that nothing visibly happens after you click "Add". Bug 1413574 has some context, but I think most of the discussion was offline.
Yeah, sorry I can't think of a good reason why they would be different. Lets match the strings as suggested.
Comment on attachment 9006661 [details] bug 1483598 - Match system and user extension add/remove page action strings r?mixedpuppy Shane Caraveo (:mixedpuppy) has approved the revision.
Attachment #9006661 - Flags: review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/3b5f59ae29ba Match system and user extension add/remove page action strings r=mixedpuppy
This wasn't the right way to fix this. Now we have two menu items that say "remove" and two menu items that say "add", as you can see in the browser.xul changes this patch made. One pair is for built-in actions, the other for extensions. The appropriate pair is selected here by setting an attribute on the context menu popup: https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser-pageActions.js#825 And then one or the other pair is hidden in CSS. Could you please file a bug to consolidate the pairs down to one and update that logic?
Ah, I saw there were different functions being called and figured we needed two. I filed bug 1488930 to clean that up.
You need to log in before you can comment on or make changes to this bug.