'CKEDITOR.dom.node.getPreviousSourceNode/getNextSourceNode' bug

[Garry Yao]

I've detected the 'previous source node' is incorrect with the attache d TC.

[Martin Kou]

#2980 may be affected by this change since it depends on the wrong behavior of getPreviousNode() - actually it tries to get the right behavior by feeding it a true argument.

[Martin Kou]

It isn't possible to walk backwards in DFS order without using lastChild.

e.g. the following script, when executed within the replacebyclass.html sample, will never give you the correct backwards walk no matter whether you feed true or false to getPreviousSourceNode():

f = frames[0];
c = l = new CKEDITOR.dom.element(f.document.links[0]).
getNextSourceNode();   // The "FCKeditor" text node
while(c = c.getPreviousSourceNode(true))
console.log(c.getOuterHtml ? c.getOuterHtml() : c.getText());

[Garry Yao]

Replying to martinkou:

#2980 may be affected by this change since it depends on the wrong behavior of getPreviousNode() - actually it tries to get the right behavior by feeding it a true argument.

This function has a lot of confusing which is error prone, and I've spotted some places which relay on the wrong behavior of this function, let's figure out and fix them after we have a common cognition with the 'correct' behavior.

[Garry Yao]

I've come with a new patch which simplified the implementation by utilize 'domWalker' and updated the TCs. Please first give a review to those TCs and then the patch.

[Garry Yao]

Replying to martinkou: I've tested with the new patch's passing your TC Martin, please reconfirm.

[Garry Yao]

Update the description since the CKEDITOR.dom.node.getNextSourceNode is not right also, we're going to fix it here.

[Garry Yao]

Fixing incorrect result when receives 'nodeType' param.

[Frederico Caldeira Knabben]

The original TCs are incorrect. Let me make some clarifications to justify it.

Suppose we have the following structure:

<p><b>A</b>B</p><div>C</div>

These two functions must return the first "touched node", from LTR or RTL, depending on the function.

So getNextSourceNode is supposed to return us the nodes in this order:

<p> -> <b> -> A -> B -> <div> -> C

getPreviousSourceNode instead is not supposed to give the exact opposite of it, but to apply the same logic, reading the code from right to left:

<div> -> C -> <p> -> B -> <b> -> A

In this case, the "touch point" of the elements are their "end boundary".

It's also quite easy to understand the why of it. In the original TCs conception, getPreviousSourceNode should return <p> by both <b> and <div>, which is wrong as we need to different results for each one of them, considering their different locations.

I've attached the correct TCs for it.

[Garry Yao]

Now I understood the semantic of 'getPreviousSourceNode' function throughly Fred, I'll update the original TCs.