1392733 - Disabled state is not synced from page action menu items to the corresponding urlbar icon

Status: RESOLVED FIXED

(Firefox :: General, defect, P1)

Description

[Tim Nguyen :ntim]

STR:
- Add "Send to device" to URL bar
- go to about:config

AR: "Send to device" is disabled in Page action menu, but not in the urlbar
ER: "Send to device" should be disabled in both

Same thing happens for "report site issue"

Comment 1

[Drew Willcoxon :adw]

Right now each page action has to take care of this by itself.  Send to Device sets the disabled attribute on its node in the panel manually, for example, and it just doesn't do that for the urlbar button.

But it would be nice if the page actions machinery would keep the states in sync.  A couple of options:

(1) Add an Action.disabled setter that updates DOM nodes.  It would have to be a setDisabledInWindow method though.

(2) Add a mutation listener on both the urlbar and panel nodes, and set disabled on one when the other changes.

Comment 2

[:Gijs (he/him)]

Or use a broadcaster for the items that need a disabled state and update the broadcaster instead?

Comment 3

[Tim Nguyen :ntim]

I've looked into the code doing this, and while doing so, I've discovered another state-sync bug between the button and the menu item: the bookmark button's tooltip isn't in sync with the menu item label (bookmark this page and Edit this bookmark).

Comment 4

[Drew Willcoxon :adw]

I see that too but I'm not sure what's wrong.  #star-button observes tooltiptext just like it does the "starred" attribute.

Comment 5

[Tim Nguyen :ntim]

(In reply to Drew Willcoxon :adw from comment #4)
> I'm not sure what's wrong.

If the page is bookmarked, you shouldn't see "Bookmark this Page" as tooltip, but "Edit This Bookmark".

Comment 6

[Drew Willcoxon :adw]

I don't know what's causing the problem, I mean.

Comment 7

[Drew Willcoxon :adw]

I filed bug 1393237 for the bookmark star tooltip problem.

Comment 8

[Drew Willcoxon :adw]

I'm adding better disabled-state handling, including syncing, as part of bug 1395387, since the WebExtensions page action API needs it.

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

Bug 1395387 is slated for 58, but we will take a more directed fix for 57 in this bug if available.

Comment 10

[:Gijs (he/him)]

Is there anything left to do here, post-bug-1395387 ?

Comment 11

[Drew Willcoxon :adw]

No, this is fixed by bug 1395387.

Comment 12

[Tim Nguyen :ntim]

An STR where this isn't fixed:

- Add Send Tab to device or Report site issue to the address bar
- Go to about:about
- Check the state of the button and the state of the menu item: they are not in sync

Comment 13

[Drew Willcoxon :adw]

Could you please open separate bugs for each of those?  The general mechanism for disabling actions is there, it's just that those two specifically haven't been updated to use it.  Send to Device just sets the "disabled" attribute on its panel button, for example.

Comment 14

[Drew Willcoxon :adw]

I filed bug 1417272 and bug 1417273 for these two.