1423677 - Toolbox breaks if the devtools.telemetry.tools.opened.version pref has invalid JSON

Status: NEW

(DevTools :: General, defect, P3)

Description

[JDev]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171128222554

Steps to reproduce:

I go to Tools -> Web Developer -> Inspector (or any similar tool)


Actual results:

A window is opened and nothing is displayed as the last state the tools were in prior to upgrade was detached.  


Expected results:

The opening should ignore the attached/detached state and always open attached due to FFQ's new paradigm.  In a profile where the last state was attached the tools open normally without any issue.

Comment 1

[Patrick Brosset <:pbro>]

Thank you for filing. Let me ask a few questions in order for me to understand the issue better:

> FFQ's new paradigm
Can you define what you mean by new paradigm here?

> The opening should ignore the attached/detached state and always open attached
Do you mean that opening the tools should always open them at the bottom of the browser (docked) instead of remembering the last setting (possibly detached as a new window)?

> A window is opened and nothing is displayed
That sounds like a really bug. So the window got opened but was completely blank?
If you manage to reproduce this issue, could you also open the browser console and see if there are errors in it (you can open it with ctrl+shift+J). If there are errors in there, can you copy them here?

Comment 2

[JDev]

(In reply to Patrick Brosset <:pbro> from comment #1)
> Thank you for filing. Let me ask a few questions in order for me to
> understand the issue better:
> 
> > FFQ's new paradigm
> Can you define what you mean by new paradigm here?

From reading issues around lost extensions like Chatzilla it was said that windowed was no longer supported

> > The opening should ignore the attached/detached state and always open attached
> Do you mean that opening the tools should always open them at the bottom of
> the browser (docked) instead of remembering the last setting (possibly
> detached as a new window)?

This is only if what I thought was the cause proves to be valid, which by your reaction it may not be.

> > A window is opened and nothing is displayed
> That sounds like a really bug. So the window got opened but was completely
> blank?
> If you manage to reproduce this issue, could you also open the browser
> console and see if there are errors in it (you can open it with
> ctrl+shift+J). If there are errors in there, can you copy them here?

Always reproducible:

SyntaxError: JSON.parse: unexpected end of data at line 1 column 1 of the JSON data
Stack trace:
logOncePerBrowserVersion@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/telemetry.js:381:21
_pingTelemetry@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/toolbox.js:658:5
open/<@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/toolbox.js:492:7
_run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:310:39
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:742:11

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to JDev from comment #2)
> > > A window is opened and nothing is displayed
> > That sounds like a really bug. So the window got opened but was completely
> > blank?
> > If you manage to reproduce this issue, could you also open the browser
> > console and see if there are errors in it (you can open it with
> > ctrl+shift+J). If there are errors in there, can you copy them here?
> 
> Always reproducible:
> 
> SyntaxError: JSON.parse: unexpected end of data at line 1 column 1 of the
> JSON data
> Stack trace:
> logOncePerBrowserVersion@resource://devtools/shared/base-loader.js ->
> resource://devtools/client/shared/telemetry.js:381:21
> _pingTelemetry@resource://devtools/shared/base-loader.js ->
> resource://devtools/client/framework/toolbox.js:658:5
> open/<@resource://devtools/shared/base-loader.js ->
> resource://devtools/client/framework/toolbox.js:492:7
> _run@resource://devtools/shared/base-loader.js ->
> resource://devtools/shared/task.js:310:39
> process@resource://gre/modules/Promise.jsm ->
> resource://gre/modules/Promise-backend.js:922:23
> walkerLoop@resource://gre/modules/Promise.jsm ->
> resource://gre/modules/Promise-backend.js:806:7
> scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm ->
> resource://gre/modules/Promise-backend.js:742:11

Link to the line that's throwing: https://dxr.mozilla.org/mozilla-release/source/devtools/client/shared/telemetry.js#381. It looks like the only consumer of this is in toolbox._pingTelemtry: https://dxr.mozilla.org/mozilla-release/source/devtools/client/framework/toolbox.js#658.

I guess the call to Services.prefs.getCharPref("devtools.telemetry.tools.opened.version") isn't a valid object. Can you see what the value of that preference is in about:config?

Also, we should consider try/catching the JSON.parse call to be safe, since this is in the startup path of the toolbox.

Comment 4

[JDev]

It's an empty string.  I changed it long long ago, I guess I never noticed the impact due to using Firebug.  The question is, why is Firefox continuing to collect telemetry data when explicitly told not to?

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to JDev from comment #4)
> It's an empty string.  I changed it long long ago, I guess I never noticed
> the impact due to using Firebug.  The question is, why is Firefox continuing
> to collect telemetry data when explicitly told not to?

The frontend calls Services.telemetry APIs unconditionally, but they won't send data if you've opted out.