Pocket button should come before bookmarks star button in urlbar

VERIFIED FIXED in Firefox 57

Status

()

defect
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: adw, Assigned: adw)

Tracking

unspecified
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 verified)

Details

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

Attachments

(1 attachment)

Assignee

Description

2 years ago
The Pocket button should come before the bookmarks star button in the urlbar.  Right now it's after.
Assignee

Updated

2 years ago
See Also: → 1387077
Iteration: --- → 57.3 - Sep 19
Flags: qe-verify+
Priority: -- → P1
QA Contact: gwimberly
Whiteboard: [photon-structure] → [reserve-photon-structure]
Comment hidden (mozreview-request)
Assignee

Comment 2

2 years ago
This is based on the patch in bug 1396053.

It adds a _urlbarInsertBeforeActionID property for actions, analogous to _insertBeforeActionID.  I'd prefer to rename the latter _panelInsertBeforeActionID, but since screenshots uses it, we'd have to go through another update just for that, which is totally not worth it, and these underscored properties are supposed to be private anyway.
Assignee

Comment 3

2 years ago
The problem is that I'm always just pushing onto this._persistedActions.idsInUrlbar, so since Pocket is in an extension and loads after the built-in actions are added, including the star button, it's added at the end of the urlbar.

Unfortunately though this patch won't fix the problem for people who are already using Nightly, since they'll have their urlbar ordering in their prefs already, and PageActions will continue to respect that.  We could version this persistent data and then bump the version for this bug, but it's not worth the complexity IMO, at least not right now, for this bug before Photon has shipped.

Comment 4

2 years ago
mozreview-review
Comment on attachment 8903863 [details]
Bug 1395743 - Pocket button should come before bookmarks star button in urlbar.

https://reviewboard.mozilla.org/r/175630/#review180976

Just so I'm clear on this, this would incorrectly order the pocket button if you removed it from the url bar and then put it back (or, if that wouldn't, if you did that with the bookmarks star) ? Or does the specified ordering take precedence over the order in which users add stuff to the url bar?

r=me either way, I think.
Attachment #8903863 - Flags: review?(gijskruitbosch+bugs) → review+
Assignee

Comment 5

2 years ago
This new property only takes effect if your profile has never seen the Pocket action before.  i.e., people upgrading to Photon, new installs.  Once PageActions has seen an action, then `shownInUrlbar` and its position in the urlbar is determined by the order of the urlbar actions stored in your prefs.  (One wrinkle though: if you uninstall (and maybe disable too?) Pocket, then it's removed from the list of "seen" actions.  So if you were to install (enable?) it after that, it'd be like brand new.)

Further, when you add an action to the urlbar, it's always appended.  (Which reminds me, it would be cool to be able to drag these things to reorder them.)

Comment 6

2 years ago
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71d59cfac7df
Pocket button should come before bookmarks star button in urlbar. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/71d59cfac7df
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I have reproduced the issue mentioned in comment 0, using an affected Firefox 57.0a1 build (BuildId:20170831220208).

I have verified that the issue is not reproducible using Firefox 57.0b6 (Build Id:20171005195903) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.