Bug 52686 - Setting outerHTML should merge text nodes
: Setting outerHTML should merge text nodes
Status: RESOLVED FIXED
: WebKit
HTML DOM
: 528+ (Nightly build)
: All All
: P3 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2011-01-18 17:00 PST by
Modified: 2011-05-03 13:47 PST (History)


Attachments
Patch (6.88 KB, patch)
2011-01-18 17:14 PST, Emil A Eklund
no flags Review Patch | Details | Formatted Diff | Diff
Patch (7.01 KB, patch)
2011-01-19 19:57 PST, Emil A Eklund
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (7.01 KB, patch)
2011-04-29 16:37 PST, Emil A Eklund
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-01-18 17:00:15 PST
When setting outerHTML adjacent text nodes should be merged. Setting outerText already has this behavior as does the IE implementation of setting outerHTML.
------- Comment #1 From 2011-01-18 17:14:40 PST -------
Created an attachment (id=79359) [details]
Patch
------- Comment #2 From 2011-01-19 00:43:10 PST -------
> This matches the behavior of outerText and the IE implementation of outerHTML.

Does HTML5 specify such behavior?
------- Comment #3 From 2011-01-19 12:33:01 PST -------
> 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.
------- Comment #4 From 2011-01-19 18:59:43 PST -------
(From update of attachment 79359 [details])
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.
------- Comment #5 From 2011-01-19 19:25:33 PST -------
(From update of attachment 79359 [details])
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.
------- Comment #6 From 2011-01-19 19:57:33 PST -------
Created an attachment (id=79543) [details]
Patch
------- Comment #7 From 2011-01-26 10:49:27 PST -------
Ping?
------- Comment #8 From 2011-04-26 15:26:19 PST -------
(From update of attachment 79543 [details])
Seems fine.
------- Comment #9 From 2011-04-26 15:27:12 PST -------
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?
------- Comment #10 From 2011-04-26 15:46:07 PST -------
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.
------- Comment #11 From 2011-04-29 16:37:24 PST -------
Created an attachment (id=91765) [details]
Patch for landing
------- Comment #12 From 2011-04-29 17:23:12 PST -------
(From update of attachment 91765 [details])
Clearing flags on attachment: 91765

Committed r85374: <http://trac.webkit.org/changeset/85374>
------- Comment #13 From 2011-04-29 17:23:18 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #14 From 2011-05-02 15:42:58 PST -------
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
------- Comment #15 From 2011-05-03 13:47:37 PST -------
Certainly. Thanks for the pointer.