Closed
Bug 1369491
Opened 7 years ago
Closed 7 years ago
Write reflow tests for the Photon menu and its subview transitions
Categories
(Firefox :: Menus, enhancement, P1)
Firefox
Menus
Tracking
()
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: mconley, Assigned: mconley)
References
Details
(Whiteboard: [photon-performance])
Attachments
(1 file)
Summary says it all.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → mconley
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Whiteboard: [photon-performance]
Updated•7 years ago
|
Iteration: --- → 55.7 - Jun 12
Flags: qe-verify-
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8873563 [details] Bug 1369491 - Write reflow tests for the new Photon appmenu and its subviews. https://reviewboard.mozilla.org/r/144948/#review148942 ::: browser/base/content/test/performance/browser_appmenu_reflows.js:113 (Diff revision 1) > + if (!navButtons) { > + return; > + } > + > + for (let button of navButtons) { > + await new Promise(resolve => setTimeout(resolve, 0)); Why do we need this? Should this be a requestAnimationFrame instead of a setTimeout(..., 0) to ensure that we are not causing several changes in a row that don't get actually displayed? Or does it not matter because of the waitForEvent promises we already await? ::: browser/base/content/test/performance/browser_appmenu_reflows.js:117 (Diff revision 1) > + for (let button of navButtons) { > + await new Promise(resolve => setTimeout(resolve, 0)); > + button.click(); > + await BrowserTestUtils.waitForEvent(PanelUI.panel, "ViewShown"); > + // Unfortunately, I can't find a better accessor to the current > + // subview, so I have to reach instance the PanelMultiView instance typo: remove the first 'instance'
Attachment #8873563 -
Flags: review?(florian) → review+
Assignee | ||
Comment 3•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8873563 [details] Bug 1369491 - Write reflow tests for the new Photon appmenu and its subviews. https://reviewboard.mozilla.org/r/144948/#review148942 > Why do we need this? Should this be a requestAnimationFrame instead of a setTimeout(..., 0) to ensure that we are not causing several changes in a row that don't get actually displayed? Or does it not matter because of the waitForEvent promises we already await? Yikes - leftovers. Will remove. Thanks!
Comment hidden (mozreview-request) |
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2b5bcdc6c93b Write reflow tests for the new Photon appmenu and its subviews. r=florian
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2b5bcdc6c93b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•7 years ago
|
Priority: -- → P1
You need to log in
before you can comment on or make changes to this bug.
Description
•