While I was working on the bug 55098, I realized that VisiblePosition can be upstream even if its affinity claims to be downstream. This bug is due to the following lines in http://trac.webkit.org/browser/trunk/Source/WebCore/editing/VisiblePosition.cpp. 456 Position candidate = position.upstream(); 457 if (candidate.isCandidate()) 458 return candidate; 459 candidate = position.downstream(); 460 if (candidate.isCandidate()) 461 return candidate; It returns an upstream position but it doesn't modify its affinity so we end up with a VisiblePosition that stores an upstream position but claims to be a downstream position.
I believe you're mixing your concepts here. Position's upstream/downstream concepts are not the same as VisiblePosition's; Upstream and downstream Positions are meant to be visually equivalent while upstream and downstream VisiblePositions explicitly are not. In other words, in the following scenario: <div>foo<br/> <span>bar</span> </div> A Position at the beginning of the second line would upstream to [span, 0] and downstream to ["bar", 0], but a VisiblePosition would jump from a downstream of [span, 0] to [br, 0].
(In reply to comment #1) > I believe you're mixing your concepts here. Position's upstream/downstream concepts are not the same as VisiblePosition's; Upstream and downstream Positions are meant to be visually equivalent while upstream and downstream VisiblePositions explicitly are not. :( I wasn't aware of this. Thanks for the clarification.
No problem. The naming is confusing, I know :(
(In reply to comment #3) > No problem. The naming is confusing, I know :( Maybe someone should do some renaming. Names that are descriptive for upstream()/ downstream() would be really verbose though I think.
(In reply to comment #4) > Maybe someone should do some renaming. Names that are descriptive for upstream()/ downstream() would be really verbose though I think. I think renaming the Position methods would be do-able. Something like next/previousVisuallyEquivalentPosition?
Okay, let's rename these two member functions.
Created attachment 85034 [details] work in progress
(In reply to comment #7) > Created an attachment (id=85034) [details] > work in progress I'm glad you went with renaming the Position, and not VisiblePosition methods. This is a good change except I'm slightly concerned with the names containing "next" and "previous" (despite that being my own proposal!). They aren't strictly next/previous, as calling the function again won't yield a further position that is also visually equivalent. They're really more like higher and lower visually equivalent boundary positions.