Consider using “Remove from Address Bar” and “Add to Address Bar” for all Page Actions

RESOLVED FIXED in Firefox 64

Status

enhancement
P3
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: e7358d9c, Assigned: mstriemer, NeedInfo)

Tracking

(Depends on 1 bug)

unspecified
mozilla64
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

9 months ago
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.
Flags: needinfo?(emanuela)
Priority: -- → P3

Comment 3

9 months ago
Let's match the built in strings.
Flags: needinfo?(emanuela)
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
Flags: needinfo?(bbell)
Flags: needinfo?(abenson)
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.

Comment 6

9 months ago
Yeah, sorry I can't think of a good reason why they would be different. Lets match the strings as suggested.
Flags: needinfo?(bbell)
Flags: needinfo?(abenson)
Assignee

Updated

9 months ago
Assignee: nobody → mstriemer
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+

Comment 9

9 months ago
Pushed by mstriemer@mozilla.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?
Flags: needinfo?(mstriemer)
Assignee

Updated

9 months ago
Blocks: 1488930
Assignee

Comment 11

9 months ago
Ah, I saw there were different functions being called and figured we needed two. I filed bug 1488930 to clean that up.

Updated

9 months ago
No longer blocks: 1488930
Depends on: 1488930

Comment 12

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3b5f59ae29ba
Status: NEW → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.