1167673 - Add nice intro/descriptive helper tooltips for the different details view tabs

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Something like the following (but better, because I can't think right now):

Waterfall: "Graph view giving a holistic overview of what Firefox is doing over time."

Call Tree: "Tabular tree view showing the aggregate hottest functions over the recording."

Flame Chart: "Flame chart of JS functions called over time."

CC'ing :wbamberg because I have a feeling he might have written very similar things already that we can copy and tweak a little if needed.

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

ni?ing, but I know he's had a busy few weeks

Comment 2

[Will Bamberg [:wbamberg]]

Sorry, I missed this. How many characters do we have? Here's my effort:

Waterfall: (Shows) The different operations the browser is performing during the recording, laid out sequentially as a waterfall.

Call Tree: Highlights bottlenecks in your JavaScript: functions where the browser spent most time during the recording.

Flame Chart: (Shows) The JavaScript call stack over the course of the recording.

Comment 3

[Giovanny Gongora [:gioyik]]

Hi,

Where is supposed to add those descriptions?

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

The markup is in browser/devtools/performance/performance.xul and locales files are in browser/locales/en-US/chrome/browser/devtools/profiler.dtd and .properties

Comment 5

[Lin Clark [:linclark]]

Attachment: Bug1167673.patch

This patch adds the tooltips that wbamberg suggested in Comment 2.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8646914 [details] [diff] [review]
Bug1167673.patch

Review of attachment 8646914 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, Lin! Looks great. Small comment on the namespacing. We have allocation views being promoted to non-experimental, so it'd be nice to have tooltip for that too. The allocations flamegraph is going away for now, so not worried about a tooltip for that.

f? Will for one more review of the verbiage (we have to update the l10n variable names if we change any text, which is a small hassle), the "JavaScript: functions" in the call tree seems strange, not sure what the ":" means in this case. Also, an idea of the allocation tooltip for the allocations call tree!

Thanks again!

::: browser/locales/en-US/chrome/browser/devtools/performance.dtd
@@ +90,5 @@
>  <!ENTITY performanceUI.options.filter.tooltiptext "Select what data to display in the timeline">
>  
> +<!-- LOCALIZATION NOTE (performanceUI.options.waterfall.tooltiptext): This is the
> +  -  tooltip for the Waterfall button. -->
> +<!ENTITY performanceUI.options.waterfall.tooltiptext "The different operations the browser is performing during the recording, laid out sequentially as a waterfall.">

We should probably use `performanceUI.views.*` as the namespace, rather than `performanceUI.options.*` to clarify the differences between these being the view buttons versus the dropdown options

Comment 7

[Will Bamberg [:wbamberg]]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #6)
> Comment on attachment 8646914 [details] [diff] [review]
> Bug1167673.patch
> 
> Review of attachment 8646914 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks, Lin! Looks great. Small comment on the namespacing. We have
> allocation views being promoted to non-experimental, so it'd be nice to have
> tooltip for that too. The allocations flamegraph is going away for now, so
> not worried about a tooltip for that.
> 
> f? Will for one more review of the verbiage (we have to update the l10n
> variable names if we change any text, which is a small hassle), the
> "JavaScript: functions" in the call tree seems strange, not sure what the
> ":" means in this case. 

The idea is that it's separating the two halves of the sentence 

"Highlights bottlenecks in your JavaScript" (...that is...) "functions where the browser spent most time during the recording"

The first part "Highlights bottlenecks in your JavaScript" is a high level description of what it shows, and the second part explains what a bottleneck is. But if it's not clear, it's not clear :). How about:

"Highlights JavaScript functions where the browser spent most time during the recording"

> Also, an idea of the allocation tooltip for the
> allocations call tree!

"Shows where your JavaScript allocated memory during the recording" ?

Comment 8

[Jordan Santell [:jsantell] [@jsantell]]

That's much more clear to me, Will, thanks!

Allocation tree tooltip looks good to me, but maybe no "your" JavaScript (because it can track gecko allocations, so not necessarily the developer's code when looking at it), does that make sense?

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

Running it by fitzgen, who's working on the allocation stuff: "Shows where memory was allocated during the recording."

Waterfall: Shows the different operations the browser is performing during the recording, laid out sequentially as a waterfall.

Call Tree: Highlights JavaScript functions where the browser spent most time during the recording.

Flame Chart: Shows the JavaScript call stack over the course of the recording.

Allocations Tree: Shows where memory was allocated during the recording.

Comment 10

[Will Bamberg [:wbamberg]]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #8)
> That's much more clear to me, Will, thanks!
> 
> Allocation tree tooltip looks good to me, but maybe no "your" JavaScript
> (because it can track gecko allocations, so not necessarily the developer's
> code when looking at it), does that make sense?

Yes, that makes sense to me.

Comment 11

[Lin Clark [:linclark]]

Attachment: Bug1167673-ns-views.patch

Thanks for the feedback :)

This patch changes the namespace to "views" and adds the allocation tab's tooltip.

Once I started reorganizing the code in performance.dtd, I had the idea that performanceUI.toolbar might be a better namespace for this. I'm going to attach another patch which uses that namespace and you can decide between the two.

Comment 12

[Lin Clark [:linclark]]

Attachment: Bug1167673-ns-toolbar.patch

As mentioned in Comment 11, this patch provides an alternate namespace, "toolbar".

Comment 13

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8647306 [details] [diff] [review]
Bug1167673-ns-toolbar.patch

Review of attachment 8647306 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great! And you're right, toolbar does sound like a better namespace. I'll merge this in shortly -- thanks for the contribution, Lin!

Comment 14

[Jordan Santell [:jsantell] [@jsantell]]

https://hg.mozilla.org/integration/fx-team/rev/d8228bc3dd80

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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