1621015 - Profiler popup button should be split into a start/stop button, and a down arrow

Status: RESOLVED FIXED

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

Description

[Greg Tatum [:gregtatum]]

Attachment: spec.png

This is a follow-up for the initial rewrite. See the spec.png image.

Comment 1

[Greg Tatum [:gregtatum]]

I spent some time looking into this today, and it looks like it'll be fairly difficult to accomplish. Here's what I've found.

Using dropmarker

There is already a dropmarker in the toolbar, like in the design spec. However, it seems to be an artifact of pre-photon code, and no existing components use it. It's embedded in the toolbarbutton, which means that the command events don't seem to map cleanly to separate buttons.

I tried augmenting the arrow key navigation, which I think is this code, to no avail.

In my initial work, I can't really get it to act like a separate button, and the DOM structure doesn't really match what we're trying to accomplilsh.

Using a custom widget

The widgets support a custom type, and this is what the buttons that are separated out use (e.g. the copy/cut/paste buttons). However, this machinery creates a completely custom widget, without support for the rest of the popup machinery. The if statement in buildWidget is where the custom widgets are implemented. The first branch delegates the button creation to the onBuild command, which returns a node. However, the second "if" branch builds the full popup button as we're currently using it. There is no existing code that is shared here. Plus the second if branch calls out to a lot of internal CustomizableUI which is hard to abstract into something shareable.

Comment 2

[Florian Quèze [:florian]]

(In reply to Greg Tatum [:gregtatum] from comment #1)

I spent some time looking into this today, and it looks like it'll be fairly difficult to accomplish. Here's what I've found.

Using dropmarker

There is already a dropmarker in the toolbar, like in the design spec. However, it seems to be an artifact of pre-photon code, and no existing components use it. It's embedded in the toolbarbutton

Bug 1640493 created a new wantdropmarker attribute that needs to be added for toolbarbuttons to still have a drop marker.

Comment 3

[Florian Quèze [:florian]]

Attachment: Bug 1621015 - Profiler popup button should be split into a start/stop button, and a down arrow, r=julienw.

Comment 4

[Florian Quèze [:florian]]

The next version of this patch will depend on the test changes in bug 1625044.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:florian, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/efa2336315ed
Profiler popup button should be split into a start/stop button, and a down arrow, r=julienw.

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/efa2336315ed