Bug 116511

Summary: Fix two assertion failures in Range::insertNode
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, darin, esprehn+autocc, tkent
Priority: P2 Keywords: BlinkMergeCandidate
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Ryosuke Niwa 2013-05-20 19:48:32 PDT 
We should probably merge https://chromium.googlesource.com/chromium/blink/+/519217f5819e51a195a79abff822474ec66d157d

> ASSERTION FAILED: childBefore == (offset ? container->childNode(offset - 1) : 0)
> third_party/WebKit/Source/WebCore/dom/RangeBoundaryPoint.h(115) : void WebCore::RangeBoundaryPoint::set(PassRefPtr<WebCore::Node>, int, WebCore::Node *)

> ASSERTION FAILED: child->parentNode()
> ../../third_party/WebKit/Source/core/dom/RangeBoundaryPoint.h(133) : void WebCore::RangeBoundaryPoint::setToBeforeChild(WebCore::Node *)
>  1   0x87594b2 WebCore::RangeBoundaryPoint::setToBeforeChild(WebCore::Node*)
>  2   0x87534a9 WebCore::Range::insertNode(WTF::PassRefPtr<WebCore::Node>, int&)

Range::insertNode calls Node::insertBefore, in which an event handler can update
the DOM structure so that RangeBoundaryPoint don't like.  We postpone event
dispatching by EventQueueScope.

Also, remove old comments about Acid3. The behavior is standardized.

We need to update fast/dom/insertBefore-refChild-crash.html because the test
caused recursive calls to the event handler. container.innerHTML='' did nothing
before this CL. Now container has the newChild because DOMNodeRemoved
event dispatching is delayed until Range::insertNode completion.

We need to update fast/text/split-text-crash.xhtml so that it doesn't stop when
the event handler is called twice. I'm not sure why the test worked before this CL.
Comment 1 Kent Tamura 2013-06-04 00:07:22 PDT 
Created attachment 203658 [details]
Patch
Comment 2 Ryosuke Niwa 2013-06-04 00:12:53 PDT 
Comment on attachment 203658 [details]
Patch

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

> Source/WebCore/ChangeLog:20
> +        Also, remove old comments about Acid3. The behavior is standardized.

Where is this new behavior specified?
Comment 3 Kent Tamura 2013-06-04 00:16:42 PDT 
Comment on attachment 203658 [details]
Patch

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

>> Source/WebCore/ChangeLog:20
>> +        Also, remove old comments about Acid3. The behavior is standardized.
> 
> Where is this new behavior specified?

Step 9 of http://dom.spec.whatwg.org/#dom-range-insertnode
Comment 4 Ryosuke Niwa 2013-06-14 21:21:56 PDT 
Comment on attachment 203658 [details]
Patch

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

>>> Source/WebCore/ChangeLog:20
>>> +        Also, remove old comments about Acid3. The behavior is standardized.
>> 
>> Where is this new behavior specified?
> 
> Step 9 of http://dom.spec.whatwg.org/#dom-range-insertnode

Please include that information in the change log.

Thanks for the fix!
Comment 5 Kent Tamura 2013-06-16 18:42:39 PDT 
Created attachment 204795 [details]
Patch for landing

Update ChangeLog
Comment 6 WebKit Commit Bot 2013-06-16 19:25:53 PDT 
Comment on attachment 204795 [details]
Patch for landing

Clearing flags on attachment: 204795

Committed r151627: <http://trac.webkit.org/changeset/151627>
Comment 7 WebKit Commit Bot 2013-06-16 19:25:55 PDT 
All reviewed patches have been landed.  Closing bug.