1691436 - Always include pageAction buttons in the address bar

Status: VERIFIED FIXED

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

Description

[Mark Striemer [:mstriemer]]

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

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1691436 - Always include pageAction buttons in the address bar.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

Comment 3

[Drew Willcoxon :adw]

Stealing this from Zibi (thanks Zibi)

Comment 4

[Drew Willcoxon :adw]

Attachment: Bug 1691436 - Always include page actions in the address bar when Proton is enabled.

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.

Comment 5

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f41ab45143320700d1905c19862f39f6442e55c4

Comment 6

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=669bfdce90c8afdd10c7193bd3f71c1ca1690615

Comment 7

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f165ef315e0c8aafa224e0bb855d97a57bd7a998

Comment 8

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e65947a65f2015e7d48049e5fee2d1a39d2c3fd

Comment 9

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d361a1c1f171f052ad1c13e3e2268487f6f1f92

Comment 10

[Drew Willcoxon :adw]

Proton disabled: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a61425cd63e6183980b86391ca7e1a9e1c3d903d
Proton enabled: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d1933d5ade75919a27f626662e97034aaab2e6c

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/003fdc737d93
Always include page actions in the address bar when Proton is enabled. r=Gijs

Comment 12

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/003fdc737d93