837735 - Remove or replace loading progress strings from the profiler

Status: VERIFIED FIXED

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

Description

[Anton Kovalyov (:anton)]

I don't think seeing "Processing worker requests..." helps web (Firefox OS) developers in any way but it might confuse them. So I propose killing these strings or replacing them with "Loading...".

Comment 1

[Anton Kovalyov (:anton)]

Attachment: Replace Cleopatra progress strings with "Loading profile..."

I basically forked enterProgressUI function and replaced text reporting with our localized string.

Comment 2

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 710442 [details] [diff] [review]
Replace Cleopatra progress strings with "Loading profile..."

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

::: browser/devtools/profiler/cleopatra/js/devtools.js
@@ +226,5 @@
> +
> +  var reporter = new ProgressReporter();
> +  reporter.addListener(function (rep) {
> +    var progress = rep.getProgress();
> +    label.innerHTML = gStrings.getStr("profiler.loading");

label.textContent = ...

do we need to change the contents of the label on every progress tick?

::: browser/locales/en-US/chrome/browser/devtools/profiler.properties
@@ +63,4 @@
>  
>  # LOCALIZATION NOTE (profiler.stop)
>  # This string is displayed on the button that stops the profiler.
> +profiler.stop=Stop

same except for whitespace, right?

Comment 3

[Anton Kovalyov (:anton)]

Thanks!

(In reply to Rob Campbell [:rc] (:robcee) from comment #2)
> Comment on attachment 710442 [details] [diff] [review]
> Replace Cleopatra progress strings with "Loading profile..."
> 
> Review of attachment 710442 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/profiler/cleopatra/js/devtools.js
> @@ +226,5 @@
> > +
> > +  var reporter = new ProgressReporter();
> > +  reporter.addListener(function (rep) {
> > +    var progress = rep.getProgress();
> > +    label.innerHTML = gStrings.getStr("profiler.loading");
> 
> label.textContent = ...
> 
> do we need to change the contents of the label on every progress tick?

You're right, we don't.

> ::: browser/locales/en-US/chrome/browser/devtools/profiler.properties
> @@ +63,4 @@
> >  
> >  # LOCALIZATION NOTE (profiler.stop)
> >  # This string is displayed on the button that stops the profiler.
> > +profiler.stop=Stop
> 
> same except for whitespace, right?

I think it shows the line as modified because of the added newline character at the end.

Comment 4

[Anton Kovalyov (:anton)]

Attachment: Replace Cleopatra progress strings with "Loading profile..."

Comment 5

[Anton Kovalyov (:anton)]

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

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

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

Comment 7

[Mihaela Velimiroviciu (:mihaelav)]

Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130311 Firefox/21.0, Build ID: 20130311042011
Mozilla/5.0 (Windows NT 6.1; rv:22.0) Gecko/20130311 Firefox/22.0, Build ID: 20130311030946

Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20130312 Firefox/21.0, Build ID: 20130312042013
Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20130312 Firefox/22.0, Build ID: 20130312031046

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20130312 Firefox/21.0, Build ID: 20130312042013
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20130307 Firefox/22.0, Build ID: 20130312031046

VErified the fix on latest Nightly and Aurora builds across platforms: "Loading profile..." message is displayed after clicking "Stop" button.