Closed Bug 1392733 Opened 7 years ago Closed 6 years ago

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


(Firefox :: General, defect, P1)




Tracking Status
firefox57 --- wontfix


(Reporter: ntim, Assigned: adw)


(Blocks 1 open bug)


(Whiteboard: [reserve-photon-structure])

- 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"
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.
Blocks: 1352697
No longer blocks: photon-structure
Assignee: nobody → ntim.bugs
Or use a broadcaster for the items that need a disabled state and update the broadcaster instead?
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).
I see that too but I'm not sure what's wrong.  #star-button observes tooltiptext just like it does the "starred" attribute.
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-structure][triage] → [reserve-photon-structure]
(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".
I don't know what's causing the problem, I mean.
I filed bug 1393237 for the bookmark star tooltip problem.
I'm adding better disabled-state handling, including syncing, as part of bug 1395387, since the WebExtensions page action API needs it.
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly
Bug 1395387 is slated for 58, but we will take a more directed fix for 57 in this bug if available.
Is there anything left to do here, post-bug-1395387 ?
Flags: needinfo?(adw)
No, this is fixed by bug 1395387.
Assignee: ntim.bugs → adw
Closed: 6 years ago
Flags: needinfo?(adw)
Resolution: --- → FIXED
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
Resolution: FIXED → ---
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.
Closed: 6 years ago6 years ago
Flags: needinfo?(ntim.bugs)
Resolution: --- → FIXED
See Also: → 1417272
See Also: → 1417273
I filed bug 1417272 and bug 1417273 for these two.
Flags: needinfo?(ntim.bugs)
You need to log in before you can comment on or make changes to this bug.