Closed Bug 1139937 Opened 7 years ago Closed 7 years ago

" TypeError: Argument 1 of Window.getComputedStyle is not an object.: CssLogic.getComputedStyle@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/styleinspector/css-logic.js:813:10" logspam in all mochitest-dt runs


(DevTools :: Inspector, defect)

Not set


(firefox38 fixed, firefox39 fixed, firefox40 fixed)

Firefox 40
Tracking Status
firefox38 --- fixed
firefox39 --- fixed
firefox40 --- fixed


(Reporter: RyanVM, Assigned: pbro)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

Forgive me if this is already on file, it's been around for awhile I could have sworn it was, but I'm not finding it now.

This happens across the board, so it adds a lot of noise the logs.
Patrick, can you help with this maybe?
Flags: needinfo?(pbrosset)
Assignee: nobody → pbrosset
Flags: needinfo?(pbrosset)
/r/6689 - Bug 1139937 - Don't try accessing the computedStyle of pseudo elements on reflow; r=miker

Pull down this commit:

hg pull -r 8729d31593600656d347c16721852cb6072fcf27
Comment on attachment 8589014 [details]
MozReview Request: bz://1139937/pbrosset

mozreview somehow didn't set R? on this review.
Attachment #8589014 - Flags: review?(mratcliffe)
Comment on attachment 8589014 [details]
MozReview Request: bz://1139937/pbrosset

Ship It!
Attachment #8589014 - Flags: review?(mratcliffe) → review+
Thank you! FYI, we'll eventually want to get this on Aurora/Beta as well.
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Please nominate this for Aurora/Beta approval when you get a chance. Thanks for fixing it!
Flags: needinfo?(pbrosset)
Comment on attachment 8589014 [details]
MozReview Request: bz://1139937/pbrosset

Approval Request Comment

[Feature/regressing bug #]: The core feature that introduced the code for this bug is bug 911209 (FF32). It added some code that runs after each layout reflow in the content page, and checks the visibility of some nodes. But that actual bug that made it possible for the exception to appear is bug 920141 (FF35), because that's when we added ::before/::after pseudo-elements to the inspector, and so, suddenly, the code that was checking visibility of nodes sometimes had to do so with pseudo-elements, and it failed at it.
My suspicion is that this was pretty rare until maybe recently when we introduced new mochitests that hit that condition. And so now, we have these exceptions in the logs.

[User impact if declined]: Not much, some users have already had these exceptions in their logs for a long time (FF35), but since that isn't breaking anything major further down the chain, it's not a problem. If declined, the exception will keep on appearing in the logs when using the inspector or when running inspector tests.

[Describe test coverage new/current, TreeHerder]: All inspector tests still pass with this fix, so we can be pretty sure that this is well covered. There's no test that asserts that the exception doesn't show in the logs though, but I'm not sure this is needed.

[Risks and why]: Low risk, just one more condition in an IF that's not on a critical path.
[String/UUID change made/needed]: None
Flags: needinfo?(pbrosset)
Attachment #8589014 - Flags: approval-mozilla-beta?
Attachment #8589014 - Flags: approval-mozilla-aurora?
Comment on attachment 8589014 [details]
MozReview Request: bz://1139937/pbrosset

Should be in 38 beta 3
Attachment #8589014 - Flags: approval-mozilla-beta?
Attachment #8589014 - Flags: approval-mozilla-beta+
Attachment #8589014 - Flags: approval-mozilla-aurora?
Attachment #8589014 - Flags: approval-mozilla-aurora+
Attachment #8589014 - Attachment is obsolete: true
Attachment #8619671 - Flags: review+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.