Closed Bug 1367012 Opened 3 years ago Closed 3 years ago

Update updateEditUIVisibility for Photon edit controls

Categories

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

53 Branch
enhancement

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.7 - Jun 12
Tracking Status
firefox55 --- fixed
firefox56 --- verified

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

(Whiteboard: [photon-structure])

Attachments

(1 file)

I missed this in the review for bug 1354108, but we should update the updateEditUIVisibility function ( https://dxr.mozilla.org/mozilla-central/rev/f9ca97a334296facd2e0ea5582e7f12d0fe70fe4/browser/base/content/browser.js#4390-4410 ) and any related tests now that we have edit controls in the Photon panel.
Flags: qe-verify+
Priority: -- → P2
Whiteboard: [photon-structure] → [photon-structure] [triage]
Whiteboard: [photon-structure] [triage] → [photon-structure]
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 55.7 - Jun 12
Priority: P2 → P1
Attachment #8875666 - Flags: review?(mdeboer)
Sorry, I just realized this isn't ready for review yet.
Comment on attachment 8875666 [details]
Bug 1367012 - update edit UI visibility checks for photon,

https://reviewboard.mozilla.org/r/147092/#review151300

Thanks for this! This must've been a bit more work than expected.

::: browser/components/customizableui/CustomizableUI.jsm:4164
(Diff revision 2)
>        let mainViewId = this._panel.querySelector("panelmultiview").getAttribute("mainViewId");
>        let mainView = doc.getElementById(mainViewId);
>        let contextMenu = doc.getElementById(mainView.getAttribute("context"));
>        gELS.addSystemEventListener(contextMenu, "command", this, true);
>        let anchor = doc.getAnonymousElementByAttribute(this._chevron, "class", "toolbarbutton-icon");
> +      this._panel.addEventListener("popupshowing", () => doc.defaultView.updateEditUIVisibility(), {once: true});

Can you add a one-line comment here to explain why this is here? Just so others will remember it too, later :)
Attachment #8875666 - Flags: review?(mdeboer) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5cb536ad2b36
update edit UI visibility checks for photon, r=mikedeboer
https://hg.mozilla.org/mozilla-central/rev/5cb536ad2b36
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Hey Gijs, any suggested STR to be able to verify this? Thanks!
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Grover Wimberly IV [:Grover-QA] from comment #8)
> Hey Gijs, any suggested STR to be able to verify this? Thanks!

Open Firefox, open customize mode, move the edit controls to the navbar.

Check they get disabled/enabled correctly based on whether you have text selected and/or focus is in an input field and/or there is stuff on the clipboard.
Then make the window narrow so they go into the dynamic portion of the overflow panel, and check the same.
Then right click the controls, use "pin to overflow menu" to put them in the permanent bit, and check that they still update correctly.

If anything seems off, compare with behaviour on release with the buttons in the overflow panel and/or hamburger panel (instead of permanent overflow panel).

Does that work? :-)
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(gwimberly)
Thanks, Gijs!

Verified on Windows, Mac, and Ubuntu.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(gwimberly)
Blocks: 1387512
You need to log in before you can comment on or make changes to this bug.