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

RESOLVED FIXED

Status

()

defect
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ntim, Assigned: adw)

Tracking

(Blocks 1 bug)

unspecified
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox57 wontfix)

Details

(Whiteboard: [reserve-photon-structure])

Reporter

Description

2 years ago
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"
Assignee

Comment 1

2 years ago
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
Reporter

Updated

2 years ago
Assignee: nobody → ntim.bugs

Comment 2

2 years ago
Or use a broadcaster for the items that need a disabled state and update the broadcaster instead?
Reporter

Comment 3

2 years ago
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).
Assignee

Comment 4

2 years ago
I see that too but I'm not sure what's wrong.  #star-button observes tooltiptext just like it does the "starred" attribute.
Status: NEW → ASSIGNED
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-structure][triage] → [reserve-photon-structure]
Reporter

Comment 5

2 years ago
(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".
Assignee

Comment 6

2 years ago
I don't know what's causing the problem, I mean.
Assignee

Comment 7

2 years ago
I filed bug 1393237 for the bookmark star tooltip problem.
Assignee

Comment 8

2 years ago
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.

Comment 10

2 years ago
Is there anything left to do here, post-bug-1395387 ?
Flags: needinfo?(adw)
Assignee

Comment 11

2 years ago
No, this is fixed by bug 1395387.
Assignee: ntim.bugs → adw
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(adw)
Resolution: --- → FIXED
Reporter

Comment 12

2 years ago
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 13

2 years ago
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.
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Flags: needinfo?(ntim.bugs)
Resolution: --- → FIXED
Assignee

Updated

2 years ago
See Also: → 1417272
Assignee

Updated

2 years ago
See Also: → 1417273
Assignee

Comment 14

2 years ago
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.