Closed Bug 1388695 Opened 7 years ago Closed 7 years ago

Add a raw payload section when looking at a non main ping in about:telemetry

Categories

(Toolkit :: Telemetry, enhancement, P1)

Product:
Toolkit
Component:
Telemetry
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: flyingrub, Assigned: flyingrub)

References

Details

Attachments

(1 file)

Bug 1388695 - Show the payload for non main ping in about:telemetry
59 bytes, text/x-review-board-request
gfritzsche
: review+
Details 
Currently we only show the general and environment data. This is confusing as we don't show the actual ping payload.
Blocks: 1384534
No longer depends on: 1384534
Summary: Add a raw payload section hen looking at a non main ping in about:telemetry → Add a raw payload section when looking at a non main ping in about:telemetry
Depends on: 1388691
Comment on attachment 8895331 [details]
Bug 1388695 - Show the payload for non main ping in about:telemetry

https://reviewboard.mozilla.org/r/166538/#review171806

::: toolkit/content/aboutTelemetry.js:2321
(Diff revision 3)
>    let elements = document.querySelectorAll(".category");
>    for (let section of elements) {
>      if (commonSections.has(section.getAttribute("value"))) {
>        continue;
>      }
> +    // only show the raw payload for non main ping

Nit: Upper-cased "Only" and end the comment with a ".".

::: toolkit/content/aboutTelemetry.xhtml:150
(Diff revision 3)
>          <p id="home-explanation"></p>
>          <p id="ping-explanation"></p>
>        </section>
>  
> +      <section id="raw-payload-section">
> +        <button id="payload-json-viewer">&aboutTelemetry.showInFirefoxJsonViewer;</button>

You should add that string to the .dtd in this patch instead of on another bug.
Attachment #8895331 - Flags: review?(gfritzsche)
Comment on attachment 8895331 [details]
Bug 1388695 - Show the payload for non main ping in about:telemetry

https://reviewboard.mozilla.org/r/166538/#review172284

::: toolkit/content/aboutTelemetry.js:996
(Diff revision 5)
>    }
>  };
>  
> +var RawPayloadData = {
> +  /**
> +   * Renders the raw pyaload

Nit: Missing trailing ".".
Attachment #8895331 - Flags: review?(gfritzsche) → review+
Keywords: checkin-needed
This has unresolved issues in MozReview. Please remember to double-check those before setting checkin-needed to avoid delays in your patches landing.
Flags: needinfo?(flyinggrub)
Flags: needinfo?(flyinggrub)
Keywords: checkin-needed
Sorry about that. I will double check next times.
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6beb0e4ddfd1
Show the payload for non main ping in about:telemetry r=gfritzsche
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6beb0e4ddfd1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment on attachment 8895331 [details]
Bug 1388695 - Show the payload for non main ping in about:telemetry

https://reviewboard.mozilla.org/r/166538/#review173130

::: toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd:24
(Diff revision 8)
>  <!ENTITY aboutTelemetry.raw "
>  Raw JSON
>  ">
>  
> +<!ENTITY aboutTelemetry.showInFirefoxJsonViewer "
> +Open in the firefox JSON viewer

You really don't want Firefox lowercase in a string.

No need to udpate the string ID, just fix the string directly.
Should I open an other issue about it ?
(In reply to flyingrub from comment #16)
> Should I open an other issue about it ?

Yes, please. On second thoughts, I'm not sure you even want to call out "Firefox" in such string?
Blocks: 1389940
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: