Closed
Bug 1395743
Opened 7 years ago
Closed 7 years ago
Pocket button should come before bookmarks star button in urlbar
Categories
(Firefox :: Address Bar, defect, P1)
Firefox
Address Bar
Tracking
()
Tracking | Status | |
---|---|---|
firefox57 | --- | verified |
People
(Reporter: adw, Assigned: adw)
References
()
Details
(Whiteboard: [reserve-photon-structure])
Attachments
(1 file)
The Pocket button should come before the bookmarks star button in the urlbar. Right now it's after.
Updated•7 years ago
|
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•7 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•7 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•7 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•7 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.)
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
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/71d59cfac7df
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment 8•7 years ago
|
||
Screenshots: https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=973e8b890a62aee4b3170558ac3b608928162ef6&newProject=mozilla-central&newRev=978d2539a8d1a49e9f9705204f3918772b337547
Comment 9•7 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•