Node::setTextContent optimization is not per spec |
|||||
Issue descriptionThere is an optimization for updating textcontent of a Node with the same string here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Node.cpp?l=1474 so the text node will be left untouched when the new text is the same as the old spec But according to https://dom.spec.whatwg.org/#dom-node-textcontent, a new text node should've been made (a different object). See also: http://jsbin.com/mahokusege/edit?html,output https://twitter.com/ziyunfei/status/724492093280804864 What is the expected result? textContent object should be a different object but with same text, and a mutation event should be fired What happens instead? textContent object is not changed at all and no mutation event is fired
,
Dec 8 2017
,
Dec 8 2017
Result of this test in some browsers: http://jsbin.com/mahokusege/edit?html,output Chrome 62.0.3202.94: Fail EdgeHTML 16.16299 Edge 41.16299.15.0: Fail Firefox 57: Pass Safari 11.0.1: Pass
,
Dec 8 2017
+rbyers@ for opinions for this: While searching the history of this change, https://codereview.chromium.org/383163003 I found the CL above added the following comment: // Note: This is an intentional optimization. // See crbug.com/352836 also. And back then, eseidel commented > I too was skeptical of this optimization. But we've had it > for years and it hasn't broken the web. We should just get > the spec changed. But the time passed, and we should focus more on interop than ever, IMO. So we have 2 options - de-optimize this and have a corresponding WPT for it - change the spec and everybody has opportunity to optimize Rick, could you advise on this?
,
Dec 8 2017
FYI, the comment above is what I stumbled upon during code review https://chromium-review.googlesource.com/c/chromium/src/+/816637
,
Dec 13 2017
Edge has this optimization too, right? I tend to agree with eseidel - that if we've never heard of any compat issue AND we have reason to believe there's non-trivial performance benefit, then we should change the spec to incorporate it. I'd suggest filing a bug on the WHATWG DOM repo, include whatever data we have (eg. do we see a benchmark like spedometer improve with this optimization enabled, if so how much) and see if we can get Gecko or WebKit to say they support changing this (breaking the two/two tie at the moment). We'd have to contribute a spec and web-platform-test PR of course.
,
Dec 14 2017
,
Sep 25
,
Oct 22
Any reason for mark it as wontfix? Thanks
,
Oct 22
@ #9, As there are no complaints from web devs aside from the example cited at the main bug about this non-spec-compliancy, I'm marking this as WontFix. Feel free to reopen this if it might be important. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by kochi@chromium.org
, Dec 8 2017Labels: Hotlist-Interop