1597378 - Create the new UI for the minimal popup/doorhanger

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), enhancement, P2)

Description

[Greg Tatum [:gregtatum] (REO Fx132)]

The new UX is much more simplified. In this bug, we can have a preffed off "hard break" in the codebase, and create a new popup/doorhanger that will interact nicely with the about:profiling page. It will not include the presets, nor the overhead information. However, it will support the profiler buttons, a link to the new about:profiler page.

Comment 1

[Greg Tatum [:gregtatum] (REO Fx132)]

Attachment: Bug 1597378 - WIP - Simplified popup

Comment 2

[Greg Tatum [:gregtatum] (REO Fx132)]

Attachment: Bug 1597378 - Change location of the presets; r?julienw

The presets in recording-utils.js couldn't be shared via the ChromeUtils.import
mechanism. When they were in recording-utils.js they could only be loaded in
via the DevTools require() loader. This commit changes it so that they are
stored in a jsm, and are also injected to each UI interface where the
ChromeUtils.import function is available (only in the initializers).

Depends on D62911

Comment 3

[Greg Tatum [:gregtatum] (REO Fx132)]

Attachment: Bug 1597378 - Remove old popup implementation; r?julienw

This commit removes the old popup implementation, but does not implement the new one
yet. This is in a follow-up commit. The client is in a bit of an awkard state where
it is mid-transition to a new architecture. Some of the code comments and documentation
were updated to reflect the changes coming up.

Depends on D62912

Comment 4

[Greg Tatum [:gregtatum] (REO Fx132)]

Attachment: Bug 1597378 - Create new UI for the profiler popup; r?julienw

This commit implements the actual UI changes. A follow-up commit adds the
tests for the changes. The CSS is a little bit awkard since it uses lots of
ID selectors rather than class selectors. I wanted to be able to write quick
selects, since it's selecting across the entire browser document. I feel
a little conflicted with the approach, as I would prefer to use classes in
general.

The panel.jsm.js file collects all of the UI handling changes rather than
having everything in menu-button.jsm.js, as the latter can get loaded
at startup. I'm not sure if it's completely worth the trouble of having
two files, as most of it should be pretty light.

This commit does not handle localization for the panel, as we should be moving
to Fluent. Rather than solve that here, I will follow-up with it in Bug 1599774.

Depends on D62913

Comment 5

[Greg Tatum [:gregtatum] (REO Fx132)]

Attachment: Bug 1597378 - Migrate profiler popup tests; r?julienw

This commit adds tests for the profiler popup, and migrates some of the
older ones to use about:profiling instead.

Depends on D62914

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b760d8fc1dc
Change location of the presets; r=julienw
https://hg.mozilla.org/integration/autoland/rev/4d1ff8bd4e64
Remove old popup implementation; r=julienw
https://hg.mozilla.org/integration/autoland/rev/b2f0cf9a387e
Create new UI for the profiler popup; r=julienw
https://hg.mozilla.org/integration/autoland/rev/38c8db44b398
Migrate profiler popup tests; r=julienw

Comment 7

[Arthur Iakab [arthur_iakab]]

Backed out 6 changesets (bug 1144438, bug 1617208, bug 1597378) for causing mochitest failures on test_perf-settings-entries.html.

Backout revision Backed out 6 changesets (bug 1144438, bug 1617208, bug 1597378) for causing mochitest failures on test_perf-settings-entries.html.

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=2181d8a99229269677715ef8b20b7d3860667004&searchStr=os%2Cx%2C10.14%2Cdebug%2Cmochitests%2Cwithout%2Ce10s%2Ctest-macosx1014-64%2Fdebug-mochitest-chrome-1proc-1%2Cm-1proc%28c1%29

Failures https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=290405620&fromchange=2181d8a99229269677715ef8b20b7d3860667004&searchStr=os%2Cx%2C10.14%2Cdebug%2Cmochitests%2Cwithout%2Ce10s%2Ctest-macosx1014-64%2Fdebug-mochitest-chrome-1proc-1%2Cm-1proc%28c1%29

Failure log https://treeherder.mozilla.org/logviewer.html#?job_id=290411296&repo=autoland

Greg can you please take a look?

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/35da034acf03
Change location of the presets; r=julienw
https://hg.mozilla.org/integration/autoland/rev/c65103d7bf0b
Remove old popup implementation; r=julienw
https://hg.mozilla.org/integration/autoland/rev/b802ce66aa59
Create new UI for the profiler popup; r=julienw
https://hg.mozilla.org/integration/autoland/rev/cc21fa09b88d
Migrate profiler popup tests; r=julienw

Comment 9

[Arthur Iakab [arthur_iakab]]

Backed out 6 changesets (bug 1144438, bug 1617208, bug 1597378) for causing devtools failures on browser_all_files_referenced.js

Backout revision https://hg.mozilla.org/integration/autoland/rev/1c55bc6e995ada5e4b6a61f0ab0ce325a5988692

Failures https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linux%2C18.04%2Cx64%2Cshippable%2Copt%2Cmochitests%2Ctest-linux1804-64-shippable%2Fopt-mochitest-devtools-chrome-e10s-1%2Cm%28dt1%29&fromchange=73d1a5e103376d0baba223a48ef1e1977d408043

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linux%2C18.04%2Cx64%2Cshippable%2Copt%2Cmochitests%2Ctest-linux1804-64-shippable%2Fopt-mochitest-devtools-chrome-e10s-1%2Cm%28dt1%29&revision=eef173a3678d6b219ba25f583434621c7b883463

Failure log https://treeherder.mozilla.org/logviewer.html#?job_id=290471398&repo=autoland

Greg can you please take a look?

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41f2f089537d
Change location of the presets; r=julienw
https://hg.mozilla.org/integration/autoland/rev/bee9d5fc55aa
Remove old popup implementation; r=julienw
https://hg.mozilla.org/integration/autoland/rev/2f59a08c4bef
Create new UI for the profiler popup; r=julienw
https://hg.mozilla.org/integration/autoland/rev/c6e4d7a385cc
Migrate profiler popup tests; r=julienw

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/41f2f089537d
https://hg.mozilla.org/mozilla-central/rev/bee9d5fc55aa
https://hg.mozilla.org/mozilla-central/rev/2f59a08c4bef
https://hg.mozilla.org/mozilla-central/rev/c6e4d7a385cc

Comment 12

[:Gijs (out for now; he/him)]

Is there a bug tracking making all this UI localizable? I was a bit surprised to find hardcoded English in the tree for this.

Comment 13

[Greg Tatum [:gregtatum] (REO Fx132)]

Yes, it's: https://bugzilla.mozilla.org/show_bug.cgi?id=1599774 which has a WIP patch attached to it.

For background, I started doing the localization in the initial patches, but the CustomizableUI didn't have Fluent in place as far as I could tell, and someone commented that I shouldn't be touching old localization files. I filed it as a follow-up, and still plan on doing the work. I spent a lot of time on this UI so far, so I'm catching up on some other projects before coming back to it. We still haven't opened up this UI path to a broader audience yet, and will have localization in place before we do. My approach was to achieve parity first with existing UI this was replacing, which also was not localized. It's definitely on my to-do list though.