[WebODF] Questions about core.PositionIterator

Friedrich W. H. Kossebau friedrich at kogmbh.com
Fri Sep 13 16:14:38 CEST 2013


Hi,

thank you Philip for your email about the point translation system, it put 
quite some light on the topic for me, and additionally also showed your POV, 
which is very valuable to have.

Still I need to grasp some things and have some questions I need you all to 
answer, let me please do it class by class. Grep for "Q:" to see where I need 
help.


So, here core.PositionIterator:

I am still puzzled about what its properties are and what states does it have. 
Let's see:

Current public methods:

// modifying
nextPosition()
previousPosition()
moveToEnd()
moveToEndOfNode(node)
setUnfilteredPosition(container, offset)

// querying
container()
domOffset()
getCurrentNode()
unfilteredDomOffset()
rightNode()
leftNode()
getPreviousSibling()
getNextSibling()

// querying, just used from tests
getNodeFilter()


So these properties:
* a filter
  (at least one for empty textnodes, but can be extended by one passed as arg)
* a root node from pure, unfiltered DOM, its subtree is where the iterator 
moves inside
* a position, defined by a container and the current position in it
Any other things queried are only derived from these core properties.
Q: Correct?
Q: What is the actual semantic of the container and the positions in it, to 
what possible points in the DOM can/should container and position point to?
Q: Most API ignores the concept of container and offset and instead talks 
about nodes/siblings. But both use the same value of the "currentPos" var. 
Confuses me completely. Why is the container concept irrelevant in those 
calls? Or do these two different views on the iterator perfectly go along? 

So the states:
The filter is at least the one for empty textnodes, so always exists and 
works.
The root node is required in the API dox, so always exists (but could be moved 
out of the DOM during the lifetime of the iterator, API dox needs to require 
that this is not happening perhaps)
The position is stored via the "walker" object and the "currentPos" variable.
The passed root node could have a subtree with no positions (after filtering). 
Q: What values would the properties have in that state?
Q: Or do we have to require that the root node allows at least one position?
Q: Should the iterator always be in positions which are allowed by the filter? 
"moveToEndOfNode()" could resolve in a position that is not valid, because of 
"walker.currentNode = node;" which will not check against the root node or the 
filter (/me to blame for that code, but noone cleaned it as well ;) ).
"setUnfilteredPosition()" is also not checking if the container is inside the 
root.
Seems to me the states of the position are not properly defined, or at least 
slightly fragile with some methods.

Questions for the implementation:
Q: Why does container() return the parentNode in case of currentPos === 0 && t 
!== Node.TEXT_NODE, and not only if t !== Node.TEXT_NODE?
Q: So what is the semantics of "container", where is the difference to 
"currentNode"?
Q: In init() why is the currentPos set to 1 if walker.firstChild() === null? 
Q: Why seems currentPos === 0 only valid with existing visible child nodes, 
what?


And than these naming issues continue to confuse me:
* getter methods named "get*" vs "*"
* getter and setter methods too similar, e.g. "nextPosition" vs. "rightNode"
* mixing of "right"/"left" and "next"/previous"
* "dom" in the "*Offset" methods is redundant, no other offset method
* "setUnfilteredPosition" sounds to me as if there is an unfiltered position 
possible and that gets set

Q: What do you think about these renamings:
// modifying
moveToNextPosition() <- nextPosition()
moveToPreviousPosition() <- previousPosition()
moveToEnd()
moveToEndOfNode(node)
setPositionByUnfilteredPosition(container, offset) <- setUnfilteredPosition(*)
(hm, ideas for better name? moveCloseToUnfilteredPosition?)

// querying
getContainer() <- container()
getOffset() <- domOffset()
getCurrentNode()
getUnfilteredOffset() <- unfilteredDomOffset()
getRightNode() <- rightNode()
getLeftNode() <- leftNode()
getPreviousSibling()
getNextSibling()

// querying, just used from tests
getNodeFilter()


Sorry, I too long postponed getting a clue of all this (again), but better now 
than never.

Cheers
Friedrich
-- 
Friedrich W. H. Kossebau // KO GmbH
http://kogmbh.com/legal/


More information about the WebODF mailing list