1357724 - [meta] Stylo: `styleSheet.cssRules` includes null rules

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P5)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

DevTools expects the rules array returned by `styleSheet.cssRules` to contain only non-null rules, but currently some appear to be null.

These leads to failures[1] in tests like "devtools/client/commandline/test/browser_cmd_csscoverage_startstop.js" such as:

Full message: TypeError: rule is null
Full stack: _getImported/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js:939:1
_run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:311: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
Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:455:5
_handleResultValue@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:387:7
_run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:319:13
TaskImpl@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:273:3
asyncFunction@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:247:14
spawn@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:161:12
_getImported@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js:933:12
_addStyleSheets/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js:915:29
_run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:311:39
TaskImpl@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:273:3
asyncFunction@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:247:14
spawn@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:161:12
_addStyleSheets@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js:888:12
StyleSheetsActor<.getStyleSheets<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js:839:26
_run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:311:39
TaskImpl@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:273:3
asyncFunction@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:247:14
handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1082:19
onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1759:15
receiveMessage@chrome://marionette/content/server.js -> resource://devtools/shared/transport/transport.js:761:7

[1]: https://treeherder.mozilla.org/logviewer.html#?job_id=92644578&repo=try&lineNumber=5233

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Clearing priority for now, so it appears on :bholley's triage, but probably P2?

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

This is because some of the rules haven't been implemented. We are closing the gap.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Also appears to affect:

* devtools/client/styleeditor/test/browser_styleeditor_import_rule.js

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

This doesn't appear to block DevTools tests anymore.  If @font-feature-values is the only one left, I would guess DevTools doesn't use that in tests.

I suppose I'll marked this fixed, since it is for DevTools tests at least.