1428244 - The attributeChangedCallback for style attribute is always fired with null as oldValue and empty string as newValue

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

(See Bug #1419322 comment #14)

This bug is fired to fix
- We are missing [CEReactions] annotation on the attributes of CSSStyleDeclaration [1].
- The attributeChangedCallback for style attribute is always fired with null as oldValue and empty string as newValue [2].

[1] https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel-cased-attribute
[2] https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=5663

Comment 1

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Another test: https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=5667

Comment 2

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Bug 1428244 - Part 1: Add [CEReactions] in CSSStyleDeclaration;

per https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel-cased-attribute.

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

Comment 3

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Bug 1428244 - Part 2: Set correct oldValue/newValue for the attributeChangedCallback which is fired from style attribute change;

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

Comment 4

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ccb9721527c460480125a7fd4dd027fede3e85ef&filter-tier=1&group_state=expanded&selectedJob=154324964

Comment 5

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

This also fixes some WPT failures in custom-elements/reactions/CSSStyleDeclaration.html.

Comment 6

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 8940109 [details]
Bug 1428244 - Part 2: Set correct oldValue/newValue for the attributeChangedCallback which is fired from style attribute change;

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

Comment 7

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bbb7b38e1ba0db44e846dbd3b632fdc20dfd0506&filter-tier=1&group_state=expanded

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8940108 [details]
Bug 1428244 - Part 1: Add [CEReactions] in CSSStyleDeclaration;

https://reviewboard.mozilla.org/r/210388/#review216616

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8940109 [details]
Bug 1428244 - Part 2: Set correct oldValue/newValue for the attributeChangedCallback which is fired from style attribute change;

https://reviewboard.mozilla.org/r/210390/#review216628

I guess we can do it this way, at least for now.

As I asked on IRC:
edgar: has anyone considered using nsIMutationObserver for CE attribute change notifications
12:19 and using its AttributeWillChange callback
12:19 That is how (DOM) MutationObserver works

Comment 10

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

(In reply to Olli Pettay [:smaug] from comment #9)
> has anyone considered using nsIMutationObserver for CE attribute
> change notifications
> 12:19 and using its AttributeWillChange callback
> 12:19 That is how (DOM) MutationObserver works

Not yet, but if it makes implementation simper and less error prone, I am happy doing that :).
Filed bug 1428784.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b89800bcb8b9
Part 1: Add [CEReactions] in CSSStyleDeclaration; r=smaug
https://hg.mozilla.org/integration/autoland/rev/ac75fed1265f
Part 2: Set correct oldValue/newValue for the attributeChangedCallback which is fired from style attribute change; r=smaug

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/b89800bcb8b9
https://hg.mozilla.org/mozilla-central/rev/ac75fed1265f

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

this wouldn't really solve the style attribute issue, since if we do this, then attribute changes coming from the parser aren't triggered.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

er, wrong bug.