Closed
Bug 1253125
Opened 7 years ago
Closed 7 years ago
[telemetry] Toolbox panels should not call toolOpened manually
Categories
(DevTools :: General, defect, P1)
DevTools
General
Tracking
(firefox47 verified)
VERIFIED
FIXED
Firefox 47
Tracking | Status | |
---|---|---|
firefox47 | --- | verified |
People
(Reporter: jryans, Assigned: jryans)
Details
(Whiteboard: [btpp-fix-now])
Attachments
(1 file)
Several panels apparently call toolOpened/Closed themselves, but they don't need to do that! The toolbox calls it for you. Only things outside of the toolbox should need to call toolOpened manually. This was spotted by QA in bug 1247985. At least the following are affected: > DEVTOOLS_SHADEREDITOR_OPENED_COUNT > DEVTOOLS_CANVASDEBUGGER_OPENED_COUNT > DEVTOOLS_MEMORY_OPENED_COUNT > DEVTOOLS_STORAGE_OPENED_COUNT > DEVTOOLS_WEBAUDIOEDITOR_OPENED_COUNT > DEVTOOLS_SCRATCHPAD_OPENED_COUNT Unfortunately, this means our opened counts and active times for these tools are not correct. :(
Assignee | ||
Updated•7 years ago
|
Flags: qe-verify+
QA Contact: mihai.boldan
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Whiteboard: [btpp-fix-now]
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=53940cc0b217 This run is noisy from a bad parent rev, but none of the failures seem related to the change here.
Assignee | ||
Comment 2•7 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/37925/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/37925/
Attachment #8726332 -
Flags: review?(mratcliffe)
Comment on attachment 8726332 [details] MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside tools. r=mratcliffe https://reviewboard.mozilla.org/r/37925/#review34543 ::: devtools/client/scratchpad/scratchpad.js (Diff revision 1) > -telemetry.toolOpened("scratchpad"); Scratchpad can be launched from the Web Developer menu or inside the toolbox. We need to find a way to only call telemetry.toolOpened and telemetry.toolClosed when we are *not* in the toolbox. Another option would be to stop automatically sending pings for main toolbox panels... the advantage being that it would be less confusing. Your choice.
Attachment #8726332 -
Flags: review?(mratcliffe)
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #3) > Comment on attachment 8726332 [details] > MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside > tools. r=mratcliffe > > https://reviewboard.mozilla.org/r/37925/#review34543 > > ::: devtools/client/scratchpad/scratchpad.js > (Diff revision 1) > > -telemetry.toolOpened("scratchpad"); > > Scratchpad can be launched from the Web Developer menu or inside the toolbox. The current patch will record `toolOpened("scratchpad")` for the toolbox panel, and `toolOpened("scratchpad-window")` for the separate. Is this sufficient, or are you saying `toolOpened("scratchpad")` should be record for both cases?
Flags: needinfo?(mratcliffe)
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4) > `toolOpened("scratchpad-window")` for the separate. for the separate *window*.
Comment on attachment 8726332 [details] MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside tools. r=mratcliffe https://reviewboard.mozilla.org/r/37925/#review34545 In that case... ship it ;)
Attachment #8726332 -
Flags: review+
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(mratcliffe)
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ad2c21a89e6e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment 9•7 years ago
|
||
I reproduced this issue on Firefox 47.0a1 (2016-03-01) and on Windows 10 x64. The issue is no longer reproducible on Firefox 47.0b3. Tests were performed on Windows 10 x64 OS and by using the affected counters from Comment 0. I am marking this issue Verified Fixed.
Updated•5 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•