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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Hiroyuki Ikezoe (:hiro)]

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

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Oops... I forgot to give them stylesheet and parent rule...

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1366657 - Set stylesheet and parent rules properly for keyframe rules.

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

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

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.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

I should have checked those tests... Thanks for filing this.

Comment 5

[Cameron McCormack (:heycam)]

Comment on attachment 8869914 [details]
Bug 1366657 - Set stylesheet and parent rules properly for keyframe rules.

https://reviewboard.mozilla.org/r/141476/#review144990

Comment 6

[Hiroyuki Ikezoe (:hiro)]

What a quick fix! Thank you Xidorn!

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

oops, we have trouble now. test_animations_omta.html has an extra failure on e10s with this patch...

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e098a6e6af06107795cf820d75f1797b2cb7ca1

Comment 9

[Hiroyuki Ikezoe (:hiro)]

I will look into the failure tomorrow.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

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.

Comment 11

[Xidorn Quan [:xidorn] UTC+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.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

(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 13

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8869914 [details]
Bug 1366657 - Set stylesheet and parent rules properly for keyframe rules.

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03691a66df90
Set stylesheet and parent rules properly for keyframe rules. r=heycam

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/03691a66df90