1367012 - Update updateEditUIVisibility for Photon edit controls

Status: VERIFIED FIXED

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

Description

[:Gijs (he/him)]

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.

Comment 1

[:Gijs (he/him)]

Attachment: Bug 1367012 - update edit UI visibility checks for photon,

Review commit: https://reviewboard.mozilla.org/r/147092/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/147092/

Comment 2

[:Gijs (he/him)]

Sorry, I just realized this isn't ready for review yet.

Comment 3

[:Gijs (he/him)]

Comment on attachment 8875666 [details]
Bug 1367012 - update edit UI visibility checks for photon,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/147092/diff/1-2/

Comment 4

[Mike de Boer [:mikedeboer]]

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 :)

Comment 5

[:Gijs (he/him)]

Comment on attachment 8875666 [details]
Bug 1367012 - update edit UI visibility checks for photon,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/147092/diff/2-3/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cb536ad2b36
update edit UI visibility checks for photon, r=mikedeboer

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5cb536ad2b36

Comment 8

[u554753]

Hey Gijs, any suggested STR to be able to verify this? Thanks!

Comment 9

[:Gijs (he/him)]

(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? :-)

Comment 10

[u554753]

Thanks, Gijs!

Verified on Windows, Mac, and Ubuntu.