Selection::validate() should be broken into smaller pieces to be less confusing I've done so. I think it's a lot easier to read. :) I also changed EState to SelectionState to match the coding style guidelines.
Created attachment 27367 [details] Minor refactoring and cleanup to Selection code WebCore/ChangeLog | 36 +++++++++++ WebCore/editing/Editor.cpp | 16 +++--- WebCore/editing/Selection.cpp | 104 +++++++++++++++++--------------- WebCore/editing/Selection.h | 36 ++++++----- WebCore/editing/SelectionController.h | 2 +- WebCore/editing/TypingCommand.cpp | 16 +++--- WebCore/page/Frame.cpp | 16 ++--- 7 files changed, 134 insertions(+), 92 deletions(-)
Created attachment 27368 [details] Slightly clearer diff, and added documentation of ASSERTS ojan and I kept hitting in this code. WebCore/ChangeLog | 36 ++++++++++++++++++++++ WebCore/editing/Editor.cpp | 16 +++++----- WebCore/editing/Selection.cpp | 54 +++++++++++++++++++------------- WebCore/editing/Selection.h | 36 ++++++++++++---------- WebCore/editing/SelectionController.h | 2 +- WebCore/editing/TypingCommand.cpp | 16 +++++----- WebCore/page/Frame.cpp | 16 ++++------ 7 files changed, 111 insertions(+), 65 deletions(-)
Adding folks who might like to review this (tiny) piece of wonderment!
Comment on attachment 27368 [details] Slightly clearer diff, and added documentation of ASSERTS ojan and I kept hitting in this code. A nit. (not that I'm a reviewer) > +++ b/WebCore/editing/Selection.cpp > +void Selection::setBaseAndExtentToDeepEquivilants() Typo: Equivalents. Here and elsewhere.
Comment on attachment 27368 [details] Slightly clearer diff, and added documentation of ASSERTS ojan and I kept hitting in this code. Will post update with spelling correction and fix of ASSERT
Created attachment 27419 [details] updated after ojan and mark's comments WebCore/ChangeLog | 36 ++++++++++++++++++++++ WebCore/editing/Editor.cpp | 16 +++++----- WebCore/editing/Selection.cpp | 52 +++++++++++++++++++------------- WebCore/editing/Selection.h | 36 ++++++++++++---------- WebCore/editing/SelectionController.h | 2 +- WebCore/editing/TypingCommand.cpp | 16 +++++----- WebCore/page/Frame.cpp | 16 ++++------ 7 files changed, 110 insertions(+), 64 deletions(-)
I've of course run the layout tests and they pass. :)
- enum EDirection { FORWARD, BACKWARD, RIGHT, LEFT }; Where did it go?
(In reply to comment #8) > - enum EDirection { FORWARD, BACKWARD, RIGHT, LEFT }; > > Where did it go? It's not needed in this file. It's already declared in SelectionController as well iirc.
Thanks! VisibleSelection here we come! Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/editing/Editor.cpp M WebCore/editing/Selection.cpp M WebCore/editing/Selection.h M WebCore/editing/SelectionController.h M WebCore/editing/TypingCommand.cpp M WebCore/page/Frame.cpp Committed r40738