1308467 - Inspector is empty when remote debugging Firefox 51 or older from Firefox 52

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Julian Descottes [:jdescottes]]

Regression from Bug 1265796.

When trying to remote debug Firefox 51 or older from Firefox 52, the inspector is blank.

The following error seems to be the root cause of the issue:

Full message: TypeError: CSS_PROPERTIES_DB.properties[name] is undefined
Full stack: normalizeCssData@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/fronts/css-properties.js:303:1
initCssProperties<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/fronts/css-properties.js:222:43

In fronts/css-properties.js we have:
> const serverDB = yield front.getCSSDatabase(getClientBrowserVersion(toolbox));

When connected to Firefox 51 or older, this returns an object with a key for each css property name (as expected) _and_ a "from" property which contains the actor id that returned the object.

This then fails when trying to add "subproperties", because we don't check that that CSS_PROPERTIES_DB.properties[name] is a valid object.

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1308467 - fix fronts/css-properties.js when remote debugging old firefox;

Review commit: https://reviewboard.mozilla.org/r/84228/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/84228/

Comment 2

[Tom Tromey :tromey]

Comment on attachment 8798846 [details]
Bug 1308467 - fix fronts/css-properties.js when remote debugging old firefox;

https://reviewboard.mozilla.org/r/84228/#review82804

Thank you.  This looks good, though one nit before landing, please.

::: devtools/shared/fronts/css-properties.js:302
(Diff revision 1)
>  
>      // Add "subproperties" information to the css properties if it's
>      // missing.
>      if (!db.properties.background.subproperties) {
>        for (let name in db.properties) {
> +        if (typeof CSS_PROPERTIES_DB.properties[name] === "object") {

I think both spots should have a short comment explaining the need for the check.

Comment 3

[Julian Descottes [:jdescottes]]

Comment on attachment 8798846 [details]
Bug 1308467 - fix fronts/css-properties.js when remote debugging old firefox;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/84228/diff/1-2/

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8798846 [details]
Bug 1308467 - fix fronts/css-properties.js when remote debugging old firefox;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/84228/diff/2-3/

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8798846 [details]
Bug 1308467 - fix fronts/css-properties.js when remote debugging old firefox;

Thanks for the review Tom.

Re-flagging for review because I had to change a bit the implementation of normalizeCssData to avoid duplicating comments. There is now a single loop and a single check for `typeof CSS_PROPERTIES_DB.properties[name] !== "object"` instead of 3.

Added a comment for the other check as well.

Comment 6

[Tom Tromey :tromey]

Comment on attachment 8798846 [details]
Bug 1308467 - fix fronts/css-properties.js when remote debugging old firefox;

https://reviewboard.mozilla.org/r/84228/#review82924

Thanks.  I think this is ok.

Comment 7

[Julian Descottes [:jdescottes]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=70f8f315e9a728a4173e62d1ec3bbb9866e357bb

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ddbdab896d8
fix fronts/css-properties.js when remote debugging old firefox;r=tromey

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9ddbdab896d8