Bug 42010 – TreeWalker::previousNode does not handle FILTER_REJECT when processing lastChild

• Component: DOM
• Status: RESOLVED FIXED

[Craig Topper]

A FILTER_REJECT response after calling acceptNode for a last child should not follow that node's last child pointer.

[Darin Adler]

Could you provide a test case?

[Darin Adler]

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.

[Craig Topper]

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.

[Darin Adler]

I see. The code should probably just say:

    if (acceptNodeResult != NodeFilter::FILTER_ACCEPT)
        break;

I think that’s right.

[Craig Topper]

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)
        break;

[Craig Topper]

Created attachment 61323 [details]
Test case

[Darin Adler]

Committed r63365: <http://trac.webkit.org/changeset/63365>

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.