1226552 - Fix the "raw ping data" string for about:telemetry

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P3)

Description

[Georg Fritzsche [:gfritzsche]]

(In reply to Francesco Lodolo [:flod] from bug 1175519, comment #20)
> Not sure I understand this string: "Raw ping data ..."
> 
> Code should *always* use "…" instead of "..." for consistency with the rest
> of the product, also not sure why the blank space before the ellipsis. Did
> it just slip through the cracks?
> 
> Given it's a matter of consistency in en-US, you can fix this without using
> a new string ID.

So, what we want here is "Raw ping data…".
The line to fix is here:
https://dxr.mozilla.org/mozilla-central/rev/a2f83cbe53ac4009afa4cb2b0b8f549289b23eeb/toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd#56

Comment 1

[Georg Fritzsche [:gfritzsche]]

Chaitanya, would you like to take this bug?

Comment 2

[chaithanya]

(In reply to Georg Fritzsche [:gfritzsche] from comment #1)
> Chaitanya, would you like to take this bug?

Yes, I would like to take this bug.

Comment 3

[chaithanya]

Attachment: Made necessary changes as per the requirements.Please let me know if there are any changes to make.

Comment 4

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8690206 [details] [diff] [review]
Made necessary changes as per the requirements.Please let me know if there are any changes to make.

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

::: toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd
@@ +52,5 @@
>  Older ping >>
>  ">
>  
> +<!ENTITY aboutTelemetry.rawPingData "
> +Raw ping data ...

This should be:
Raw ping data…

Also, this entry should already exist, maybe you have to update your clone of the source code to see it?

Comment 5

[chaithanya]

Attachment: Made changes as by comment4. Please let me know if there are any more changes to make

Comment 6

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8691390 [details] [diff] [review]
Made changes as by comment4. Please let me know if there are any more changes to make

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

::: toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd
@@ +52,5 @@
>  Older ping >>
>  ">
>  
>  <!ENTITY aboutTelemetry.rawPingData "
> +Raw ping data...

This uses three dot characters, "...".
Francesco requested that we use a single "…" character here instead.

Comment 7

[chaithanya]

Attachment: Made changes as by comment6. Please let me know if there are any more changes to make

Comment 8

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8692546 [details] [diff] [review]
Made changes as by comment6. Please let me know if there are any more changes to make

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

Thanks, this looks good now!
Please upload this with a good commit message, e.g.:
> Fix the "raw ping data" string for about:telemetry. r=gfritzsche

... then we can get this landed.

Comment 9

[chaithanya]

Attachment: Made changes as by comment8. Please let me know if there are any more changes to make

Comment 10

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8695925 [details] [diff] [review]
Made changes as by comment8. Please let me know if there are any more changes to make

This looks good now, thanks!
Sorry this took a while to review (was busy with an all-hands meeting last week).

Comment 11

[Georg Fritzsche [:gfritzsche]]

As this is a string-only change, no try run is needed before landing.

Comment 12

[Carsten Book [:Tomcat]]

failed to apply:

renamed 1226552 -> Fix_the_raw_ping_data_string_for_about_telemetry_r=gfritzscherawping.patch
applying Fix_the_raw_ping_data_string_for_about_telemetry_r=gfritzscherawping.patch
patching file toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd
Hunk #1 FAILED at 47
1 out of 1 hunks FAILED -- saving rejects to file toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh Fix_the_raw_ping_data_string_for_about_telemetry_r=gfritzscherawping.patch

Comment 13

[Georg Fritzsche [:gfritzsche]]

Chaitanya, can you update to the newest mozilla-central and see that the patch still applies?

Comment 14

[chaithanya]

(In reply to Georg Fritzsche [:gfritzsche] [away Dec 19 - Jan 3] from comment #13)
> Chaitanya, can you update to the newest mozilla-central and see that the
> patch still applies?

Yes sure

Comment 15

[chaithanya]

(In reply to Georg Fritzsche [:gfritzsche] [away Dec 19 - Jan 3] from comment #13)
> Chaitanya, can you update to the newest mozilla-central and see that the
> patch still applies?

Yes, the patch still applies.

Comment 16

[chaithanya]

Attachment: Made changes as by comment 13. Hope it works clean.

Comment 17

[chaithanya]

Attachment: Added r=<reviewer>

Comment 18

[Alessio Placitelli [:Dexter]]

Comment on attachment 8702632 [details] [diff] [review]
Added r=<reviewer>

That was a rebase, so no need to request a review again.

The commit message is missing the bug number, but I'll take care of that when landing.

Comment 19

[Alessio Placitelli [:Dexter]]

https://hg.mozilla.org/integration/fx-team/rev/189cbfa99950a419e8be5a38489793ca5b9b641c
Bug 1226552 - Fix the raw ping data string for about:telemetry. r=gfritzsche

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/189cbfa99950