Bug 42010 - TreeWalker::previousNode does not handle FILTER_REJECT when processing lastChild
Summary: TreeWalker::previousNode does not handle FILTER_REJECT when processing lastChild
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
Depends on:
Reported: 2010-07-09 17:33 PDT by Craig Topper
Modified: 2019-02-06 09:04 PST (History)
2 users (show)

See Also:

Test case (1.34 KB, text/html)
2010-07-12 21:57 PDT, Craig Topper
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Craig Topper 2010-07-09 17:33:40 PDT
A FILTER_REJECT response after calling acceptNode for a last child should not follow that node's last child pointer.
Comment 1 Darin Adler 2010-07-12 14:49:00 PDT
Could you provide a test case?
Comment 2 Darin Adler 2010-07-12 14:55:50 PDT
From code inspection, I’m not sure I see the problem. The logic looks right to me. I’ll need to see the test case to understand what is actually wrong.
Comment 3 Craig Topper 2010-07-12 15:57:09 PDT
I have a test case, but I'm at work right now and don't have access to it. But I can attempt to better explain the problem

In the following code, a FILTER_REJECT response should terminate the loop instead of going back around. If you go back around you're asking for the lastChild of a node that was just rejected.

222	            while (Node* lastChild = node->lastChild()) {
223	                node = lastChild;
224	                acceptNodeResult = acceptNode(state, node.get());
225	                if (state && state->hadException())
226	                    return 0;
227	                if (acceptNodeResult == NodeFilter::FILTER_ACCEPT)
228	                    continue;
229	            }

Also the "if (acceptNodeResult == NodeFilter::FILTER_ACCEPT)" part of the loop is pointless as it just does a continue which is exactly what would happen if it weren't there at all.
Comment 4 Darin Adler 2010-07-12 16:07:19 PDT
I see. The code should probably just say:

    if (acceptNodeResult != NodeFilter::FILTER_ACCEPT)

I think that’s right.
Comment 5 Craig Topper 2010-07-12 16:08:35 PDT
You don't want to do that because FILTER_SKIP would still allow you to see the children. So it should be 

    if (acceptNodeResult == NodeFilter::FILTER_REJECT)
Comment 6 Craig Topper 2010-07-12 21:57:28 PDT
Created attachment 61323 [details]
Test case
Comment 7 Darin Adler 2010-07-14 15:51:08 PDT
Committed r63365: <http://trac.webkit.org/changeset/63365>
Comment 8 Lucas Forschler 2019-02-06 09:04:14 PST
Mass moving XML DOM bugs to the "DOM" Component.