Bug 52686 – Setting outerHTML should merge text nodes

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

[Emil A Eklund]

When setting outerHTML adjacent text nodes should be merged. Setting outerText already has this behavior as does the IE implementation of setting outerHTML.

[Emil A Eklund]

Created attachment 79359 [details]
Patch

[Alexey Proskuryakov]

> This matches the behavior of outerText and the IE implementation of outerHTML.

Does HTML5 specify such behavior?

[Emil A Eklund]

> Does HTML5 specify such behavior?

No, HTML5 doesn't specify how text nodes should be handled when setting outerHTML and doesn't cover outerText at all.

[Adam Barth]

Comment on attachment 79359 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79359&action=review

> Source/WebCore/html/HTMLElement.cpp:386
>      HTMLElement* parent = static_cast<HTMLElement*>(p);

We don't need to hold a reference to parent here?  It's strange that we re-check parentNode for null below but don't hold a ref here.

[Emil A Eklund]

Comment on attachment 79359 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79359&action=review

>> Source/WebCore/html/HTMLElement.cpp:386

> 
> We don't need to hold a reference to parent here?  It's strange that we re-check parentNode for null below but don't hold a ref here.

Good point, I'll change it to hold a reference to the parent node instead.

[Emil A Eklund]

Created attachment 79543 [details]
Patch

[Emil A Eklund]

Ping?

[Tony Chang]

Comment on attachment 79543 [details]
Patch

Seems fine.

[Tony Chang]

Oh, are we worried that we don't collapse chains of text nodes or should we not get into that state because we collapse at every change?  What does IE do in this case?

[Emil A Eklund]

We shouldn't be able to get into that state. If you can think of any case where we could I'd love to hear about it!

IE always collapses adjacent text nodes.

[Emil A Eklund]

Created attachment 91765 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 91765 [details]
Patch for landing

Clearing flags on attachment: 91765

Committed r85374: <http://trac.webkit.org/changeset/85374>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Aryeh Gregor]

Could you please file bugs against HTML5 when you find problems with it, instead of just changing behavior and letting the spec remain incorrect?  You don't have to make an account or anything.  Just go to http://whatwg.org/html5/, find the right part of the spec, and file a comment in the little box at the bottom.

I just filed a bug: http://www.w3.org/Bugs/Public/show_bug.cgi?id=12584

[Emil A Eklund]

Certainly. Thanks for the pointer.