Bug 116511 - Fix two assertion failures in Range::insertNode
Summary: Fix two assertion failures in Range::insertNode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords: BlinkMergeCandidate
Depends on:
Blocks:
 
Reported: 2013-05-20 19:48 PDT by Ryosuke Niwa
Modified: 2013-06-16 19:25 PDT (History)
5 users (show)

See Also:


Attachments
Patch (8.38 KB, patch)
2013-06-04 00:07 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch for landing (8.46 KB, patch)
2013-06-16 18:42 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.