Closed Bug 1691436 Opened 4 years ago Closed 4 years ago

Always include pageAction buttons in the address bar

Categories

(Firefox :: Toolbars and Customization, enhancement, P2)

enhancement
Points:
3

Tracking

()

VERIFIED FIXED
88 Branch
Iteration:
88.2 - Mar 8 - Mar 21
Tracking Status
firefox88 --- verified

People

(Reporter: mstriemer, Assigned: adw)

References

(Blocks 1 open bug)

Details

(Whiteboard: [proton-address-bar])

Attachments

(1 file, 1 obsolete file)

All pageAction buttons should be included in the address bar.

This can be achieved by updating the pinnedToUrlBar property [1] to always return true. There is some state management code in PageActions.jsm so there could be some better way of doing this but it looks like this could be a simple way to get started.

The bookmark button should always be the last button on the address bar. Changing the pinnedToUrlBar logic to always return true appears to break this functionality.

[1] https://searchfox.org/mozilla-central/rev/d537e47349944c0fbd0100bd52c30e493e748c2e/browser/modules/PageActions.jsm#692

Blocks: 1691454
Component: Address Bar → Toolbars and Customization
Points: --- → 2
Keywords: helpwanted
Assignee: nobody → zbraniecki
Status: NEW → ASSIGNED

I can add a test for that but first wanted to verify that I'm doing the right change here.

Stealing this from Zibi (thanks Zibi)

Assignee: zbraniecki → adw
Iteration: --- → 88.2 - Mar 8 - Mar 21
Points: 2 → 3
Keywords: helpwanted

The IDs of actions pinned to the urlbar are kept in
PageActions._persistedActions.idsInUrlbar. To pin all actions to the urlbar,
theoretically all we need to do is add all action IDs to this array, and things
should just work. PageActions already has a migration mechanism that could check
if Proton is enabled and add all the action IDs if it is. However, that would
mean that if Proton were subsequently disabled, or if the user downgraded to a
Firefox without Proton, they'd end up with all their actions still in the
urlbar. They could remove them one by one so it's not a big problem, but it
would be annoying.

Instead, this patch keeps two arrays of urlbar IDs. One stashes the user's
original pinned actions and PageActions never touches it as long as Proton is
enabled, and the other is the usual array that happens to have the IDs of all
the actions when Proton is enabled.

When PageActions initializes _persistedActions on init (by either loading it
from prefs or using the default value), it moves idsInUrlbar to
_idsInUrlbarNonProton and then sets idsInUrlbar to an empty array. When
PageActions subsequently registers all the built-in actions, it forces
action._pinnedToUrlbar to true instead of computing it from idsInUrlbar, and
since pinnedToUrlbar is true, all actions end up in idsInUrlbar. PageActions
works as it usually does after that, except that when it stores
_persistedActions back to prefs, it moves _idsInUrlbarNonProton back to
idsInUrlbar, just for the store.

Ultimately we need to redesign this module. We can remove a lot of it since
Proton removes the panel, and we don't need the idsInUrlbar array at all since
now all actions must be in the urlbar.

Attachment #9207551 - Attachment is obsolete: true
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/003fdc737d93 Always include page actions in the address bar when Proton is enabled. r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: