Remove the domWalker class

[Frederico Caldeira Knabben]

The CKEDITOR.dom.walker class has been introduced to replace the CKEDITOR.dom.domWalker class. Now, any part of the code using the domWalker must be changed to use walker instead, and domWalker is to be delete.

[Garry Yao]

The new walker class 'CKEDITOR.dom.walker' been introduced at #2876 has changed the way of reverse traversing in the dom tree, comparing of the two algorithms:

1. CKEDITOR.dom.domWalker is using exactly the reverse sequence of depth-first, pre-order traversal.
2. CKEDITOR.dom.walker is using right-to-left, post-order traversal which is same as the one used by CKEDITOR.dom.node::getPreviousSourceNode.

While the new way of traversing satisfies many use-case like boundary checking logic within CKEDITOR.dom.range , but the original 'reverse source order' way is somehow still needed for certain cases like whole-word checking logic within find plugin, take the following example:

	1<p>^2^</p>

with the new walker logic, function CKEDITOR.dom.walker::previous will report text node of '1' as result, obviously the result is not good for determining whether text node of '2' is a whole-word matching. So I guess we still need to preserve the original logic of walker regard reverse traversal in some way.

[Frederico Caldeira Knabben]

It's interesting to see... Martin got that first impression also.

It's also possible to do that by using the "guard" function. Check out the iterator code of the new walker class. It passes the guard function to getPreviousSourceNode and getNextSourceNode, which is then executed for every up and down (and match) in the walk.

[Garry Yao]

The patch is containing the latest patch of #3313 which it depends on for easy reviewing.

[Garry Yao]

New patch comes with the following changes:

1. Update with current turnk;
2. Include the emergent part of the fixing on #3051 which has been defered.

[Martin Kou]

Two feature problems:

1. Many commands stopped working after the patch (e.g. list command)
2. Match cyclic stopped working

[Garry Yao]

Sorry, I've been forgetting to include certain changes to walker.js which is required here:

1. walker should by default avoid using 'startFromSibling' to get next source node;
2. augmenting CKEDITOR.dom.element::isBlockBoundary here as within the old domwalker.js.

[Martin Kou]

It is still not working - anything that involves the DOM iterator seems to break paragraphs into pieces now.

e.g.

1. Open replacebyclass.html.
2. Put caret in the middle of the default paragraph.
3. Press "Insert Numbered List" button.
4. The paragraph is broken into two pieces.

[Martin Kou]

But on the other hand, the issue with the find dialog seems to be fixed.

[Garry Yao]

I've found a new possibility to further simplify the enlarge by block unit powered by the new 'walker', since there's no standalone TCs for enlarge block function, I've tested it with the domiterator test suite.

[Garry Yao]

I've found that we can't rely on CKEDITOR.dom.range::getBoundaryNodes any more for marking boundary, in case of the following range:

^<p id="p1">paragraph<p>^

The function will report (startNode:p1, endNode:p1) as the result, it's right from the semantic perspective, but this info could never be used as position marker, since it's ambigulous for the walker to know whether it's at the beginning OR at the end, if the walker start with this start/end node, both call of 'waker.next()' and 'walker.previous()' will return the same result.
Currently I found a workaround by using bookmark nodes to establish a pseudo margin, like:

<span _fck_bookmark="1" /><p id="p1">paragraph<p><span _fck_bookmark="1"><span _fck_bookmark="1" />

So the walker is quite cleared about the position and it could walk back and forth at will.

[Frederico Caldeira Knabben]

We're introducing important changes to the dom.walker class with #3367. I'm freezing this ticket until that one don't get concluded.

[Garry Yao]

The patch is including the latest patch from #3477 for easy reviewing.

[Martin Kou]

Still Review-

The new DOM range enlarge API isn't working.

For CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS, it's not including the ending <BR> node. This is something being expected by other parts of the code (e.g. the list plugin).

For CKEDITOR.ENLARGE_BLOCK_CONTENTS, it seems to be halting incorrectly in front of <BR> nodes, and also it seems to be not valid for empty blocks.

[Garry Yao]

Fixing the above TCs.

[Martin Kou]

The bugs with CKEDITOR.ENLARG_BLOCK_CONTENTS are still not fixed.

btw I'm actually not too sure about test_enlarge_block5... coz there're a few possible ways for a range to select a block. So even if it fails, as long as it's selecting the empty block, it's ok.

[Garry Yao]

Replying to martinkou:

The bugs with CKEDITOR.ENLARG_BLOCK_CONTENTS are still not fixed.

Sorry for missing the enlarge(blockunit) TCs last time, they're really nice, helping to reveal some hideous bug inside walker.

[Garry Yao]

Fixed with [3471].