Bug 166716 – cssText setter should change style attribute when the serialization differs

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Krzysztof]

When two nodes with the same style attribute (in terms of CSS not string value equality) are compared the isEqualNode returns false if the style attribute string values (el.style.cssText) are not the same which is valid according to specs. However, if those values gets normalized and updated via el.style.cssText, isEqualNode still returns false (instead of true).

If el.setAttribute( 'style', ... ) is used, everything works as expected.

It only happens if cssText is updated with the same value as style attribute already has (so no change in CSS meaning, just normalization.)


Steps to Reproduce:
I created a CodePen snippet which illustrates the issue: http://codepen.io/f1ames/pen/mRdWbQ.

Expected Results:
The isEqualNode method should return true after styles normalization updated via el.style.cssText for the same nodes.

Actual Results:
The isEqualNode method returns false after styles normalization updated via el.style.cssText for the same nodes.


Version:
Safari: Version 10.0.1 (11602.2.14.0.7)
OS X El Captain: Version 10.11.6

[Radar WebKit Bug Importer]

<rdar://problem/29888665>

[Simon Fraser (smfr)]

rdar://problem/29861252

[Ahmad Saleem]

Created attachment 459833 [details]
Safari 15.4 matches with Firefox & Chrome on Test Case

It seems that Safari is matching with other browsers now in output. Unless all browsers are wrong and I am testing it incorrectly.

[Alexey Proskuryakov]

I am getting different results from Safari and Chrome on the snippet. In Safari, the JS console has false, false, true; in Chrome, it's false, true, true.

Looks like this could be a problem for attached event listeners.

[Karl Dubost]

false, true,  true  firefox and chrome
false, false, true  safari


The initial html is
<b style="color: red; width: 10px">a</b>
<b style="width:10px;color:red;">a</b>

The test with different results:

var el1 = document.querySelector( 'b' ),
    el2 = document.querySelectorAll( 'b' )[ 1 ];

el1.style.cssText = 'color:red;width:10px;';
el2.style.cssText = 'color:red;width:10px;';

console.log( el1, el2, el1.isEqualNode( el2 ) );

https://dom.spec.whatwg.org/#ref-for-dom-node-isequalnode①
> The isEqualNode(otherNode) method steps 
> are to return true if otherNode is 
> non-null and this equals otherNode; 
> otherwise false.


The comparison algorithm is:
https://dom.spec.whatwg.org/#concept-node-equals


The strange thing is that the developer tools are reporting weird things. Like if the console.log was happening at the end and everything was already set with color: blue.

Check the console for this codepen instead.


https://codepen.io/webcompat/pen/NWMXjxX?editors=1012

el2.style.cssText
el2.attributes.style

return two different things.

[Anne van Kesteren]

Looking at

http://software.hixie.ch/utilities/js/live-dom-viewer/?%3Cb%20style%3D%22color%3A%20red%3B%20width%3A%2010px%22%3Ea%3C%2Fb%3E%0A%3Cb%20style%3D%22width%3A10px%3Bcolor%3Ared%3B%22%3Ea%3C%2Fb%3E%0A%3Cscript%3E%0Avar%20el1%20%3D%20document.querySelector(%20%27b%27%20)%2C%0A%20%20%20%20el2%20%3D%20document.querySelectorAll(%20%27b%27%20)%5B%201%20%5D%3B%0A%0Ael1.style.cssText%20%3D%20%27color%3Ared%3Bwidth%3A10px%3B%27%3B%0Ael2.style.cssText%20%3D%20%27color%3Ared%3Bwidth%3A10px%3B%27%3B%0A%0Aw(el1.isEqualNode(%20el2%20)%20)%3B%0A%0Aw(el1.getAttribute(%22style%22))%3B%0Aw(el2.getAttribute(%22style%22))%3B%0A%3C%2Fscript%3E

it seems that the problem here is that initially the first element's style attribute does not have a trailing semicolon. And then the cssText setter does not end up changing the contents of the style sheet we probably optimize and return early whereas other browsers do a set with the serialization (and thus add the semicolon).

If I manually add the semicolon the nodes are equal.

Fixing this might impact performance benchmarks.

[Anne van Kesteren]

Removing `bool changed =` from

PropertySetCSSStyleDeclaration::setCssText()

and assuming it's always true would fix this.

[Anne van Kesteren]

Pull request: https://github.com/WebKit/WebKit/pull/22022

[EWS]

Committed 276176@main (9494ab07d911): <https://commits.webkit.org/276176@main>

Reviewed commits have been landed. Closing PR #22022 and removing active labels.