Update updateEditUIVisibility for Photon edit controls

VERIFIED FIXED in Firefox 55

Status

()

enhancement
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

(Blocks 1 bug)

53 Branch
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed, firefox56 verified)

Details

(Whiteboard: [photon-structure])

Attachments

(1 attachment)

Assignee

Description

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

Updated

2 years ago
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
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8875666 - Flags: review?(mdeboer)
Assignee

Comment 2

2 years ago
Sorry, I just realized this isn't ready for review yet.
Comment hidden (mozreview-request)

Comment 4

2 years ago
mozreview-review
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+
Comment hidden (mozreview-request)

Comment 6

2 years ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5cb536ad2b36
update edit UI visibility checks for photon, r=mikedeboer

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5cb536ad2b36
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Comment 8

2 years ago
Hey Gijs, any suggested STR to be able to verify this? Thanks!
Flags: needinfo?(gijskruitbosch+bugs)
Assignee

Comment 9

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

Comment 10

2 years ago
Thanks, Gijs!

Verified on Windows, Mac, and Ubuntu.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(gwimberly)

Updated

2 years ago
Blocks: 1387512
You need to log in before you can comment on or make changes to this bug.