stylo: NS_ERROR_NOT_AVAILABLE occurs when modifying style for a keyframe rules obtained by cssom

RESOLVED FIXED in Firefox 55

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: hiro, Assigned: xidorn)

Tracking

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

8 months ago
When running test_animations.html, we get NS_ERROR_NOT_AVAILABLE when modifying keyframe style [1]

 var dyn2_kf1 = dyn2.cssRules[0]; // currently 0% { margin-left: 100px }         
 dyn2_kf1.style.marginLeft = "-100px"; 

The error comes from nsDOMCSSDeclaration::ModifyDeclaration [2], it seems the ServoKeyframeRule has no mStyleSheet at that time.

[1] https://hg.mozilla.org/mozilla-central/file/9851fcb0bf4d/layout/style/test/test_animations.html#l1555
[2] https://hg.mozilla.org/mozilla-central/file/9851fcb0bf4d/layout/style/nsDOMCSSDeclaration.cpp#l334
Oops... I forgot to give them stylesheet and parent rule...
Assignee: nobody → xidorn+moz
Comment hidden (mozreview-request)
According to my local test, it would bumps failures on test_animations.html from 3 to 5, and there is no longer any js exception. I guess that indicates it is fixed by this simple patch.
I should have checked those tests... Thanks for filing this.
Comment on attachment 8869914 [details]
Bug 1366657 - Set stylesheet and parent rules properly for keyframe rules.

https://reviewboard.mozilla.org/r/141476/#review144990
Attachment #8869914 - Flags: review?(cam) → review+
(Reporter)

Comment 6

8 months ago
What a quick fix! Thank you Xidorn!
oops, we have trouble now. test_animations_omta.html has an extra failure on e10s with this patch...
(Reporter)

Comment 9

8 months ago
I will look into the failure tomorrow.
Flags: needinfo?(hikezoe)
(Reporter)

Comment 10

8 months ago
OK, I check the results of test_animations_omta.html locally.  It turns out that the results are originally different on e10s and non-e10s without attachment 8869914 [details].  It's due to bug 1361938.  On current mozilla-central the failure number are coincidentally the same.
Depends on: 1361938
Flags: needinfo?(hikezoe)
Thanks for the help. I guess I may land this patch with that specific test disabled with a comment referring to bug 1361938 rather than waiting for that to land, because if that bug ends up not fixing all failures, it would probably make landing patches more complicated. Probably marking that test with [*] would be enough.
(Reporter)

Comment 12

8 months ago
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #11)
> Thanks for the help. I guess I may land this patch with that specific test
> disabled with a comment referring to bug 1361938 rather than waiting for
> that to land, because if that bug ends up not fixing all failures, it would
> probably make landing patches more complicated. Probably marking that test
> with [*] would be enough.

Sounds reasonable,  I forget about the [*].  The mechanism what you did is quite useful. ;-)
Comment hidden (mozreview-request)

Comment 14

8 months ago
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/03691a66df90
Set stylesheet and parent rules properly for keyframe rules. r=heycam

Comment 15

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/03691a66df90
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.