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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 21 11:49:00 PDT 2021


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

--- Comment #3 from Julian Gonzalez <julian_a_gonzalez at apple.com> ---
(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.

-- 
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/8ac972ce/attachment.htm>


More information about the webkit-unassigned mailing list