[Webkit-unassigned] [Bug 224836] Crash in StyledMarkupAccumulator::traverseNodesForSerialization()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=224836

--- Comment #4 from Julian Gonzalez <julian_a_gonzalez at apple.com> ---
(In reply to Julian Gonzalez from comment #3)
> (In reply to Ryosuke Niwa from comment #2)
> > Comment on attachment 426604 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=426604&action=review
> > 
> > > Source/WebCore/editing/markup.cpp:713
> > > +        bool aboutToGoPastEnd = pastEnd && isDescendantOf(*pastEnd, *n) && !next;
> > > +        if (aboutToGoPastEnd)
> > 
> > This isn't quite right. When pastEnd && isDescendantOf(*pastEnd, *n) is true,
> > we want to set next regardless of whether next is null or not when enterNode
> > returned false.
> > We currently don't hit this case because canonicalization of position
> > will mostly avoid that situation to arise but I don't think we want to rely
> > on that.
> > 
> > The case we care about is when both of the above conditions were false.
> > In that case, we've entered a node and it has children so we don't want to
> > skip them here.
> > 
> > So, we probably want to define a new boolean indicating condition like this:
> > 
> > bool didEnterNode = false;
> > if (!enterNode(*n))
> >     next = nextSkippingChildren(*n);
> > else if (!hasChildNodes(*n))
> >     exitNode(*n);
> > else
> >     didEnterNode = true;
> > bool aboutToGoPastEnd = pastEnd && (isDescendantOf(*pastEnd, *n) || (!next
> > && !didEnterNode));
> 
> I don't think this is quite right either, as this approach breaks several
> pasteboard tests:
> 
> [1286/1900] editing/pasteboard/paste-4039777-fix.html failed unexpectedly
> (text diff)
> [1450/1900] editing/pasteboard/paste-table-001.html failed unexpectedly
> (text diff)
> [1471/1900] editing/pasteboard/paste-text-003.html failed unexpectedly (text
> diff)
> [1599/1900]
> editing/pasteboard/simplfiying-markup-should-not-strip-content.html failed
> unexpectedly (text diff)
> [1641/1900] editing/pasteboard/testcase-9507.html failed unexpectedly (text
> diff)
> 
> I think this makes sense, considering that we don't necessarily want to stop
> if the last node is a descendent of n if it's later on.

Looks like

bool aboutToGoPastEnd = pastEnd && !didEnterNode && (isDescendantOf(*pastEnd, *n) || !next);

works here.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210421/4fdc0e12/attachment.htm>