RESOLVED FIXED 6523
Extract data from SelectionController 
https://bugs.webkit.org/show_bug.cgi?id=6523
Summary Extract data from SelectionController
Duncan Wilcox
Reported 2006-01-13 06:06:35 PST
The attached patch refactors the data portions of SelectionController into its own class, "Selection". The extracted data are the base/extent/start/end positions and cursor affinity. Also a couple cached values generated by validation are kept around in the new class. This patch does away with all the different affinities (start/end/base/extent) that were never implemented properly, since affinity only makes sense when the selection is a caret (as per discussion with mjs and darin on #webkit). This is a step in the development of bug 4582.
Attachments
patch (104.57 KB, patch)
2006-01-13 06:16 PST, Duncan Wilcox 		darin: review-
DetailsFormatted DiffDiff
patch (105.15 KB, patch)
2006-01-13 16:53 PST, Duncan Wilcox 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Duncan Wilcox
Comment 1 2006-01-13 06:16:45 PST
Created attachment 5642 [details] patch
Darin Adler
Comment 2 2006-01-13 07:56:37 PST
Comment on attachment 5642 [details] patch Please don't landed commented out lines like these: +//#include <qrect.h> +//#include "editing/visible_text.h" I don't think Selection.h needs to include "misc/shared.h". For files like Selection.cpp we now are back to "using namespace DOM" rather than individual "using" lines. The copy constructor and assignment operator that are implemented for Selection do exactly what the defaults would do. In those cases it's better to not declare or define them and let the compiler take care of it. Since Selection.h includes things like PassRefPtr.h and dom_position.h, those includes can be removed from the top of SelectionController. - case SelectionController::RANGE: + case khtml::Selection::RANGE: Why do we need a khtml:: prefix in the above code and other lines like it? This seems like a nice next step. Over time we might refine and adjust what Selection has vs. SelectionController, and change various clients that hold SelectionController to hold Selection instead. The above are minor quibbles but lets get answers to them before we do review+. Did you run layout tests? Do they all give identical results?
Duncan Wilcox
Comment 3 2006-01-13 16:53:05 PST
Created attachment 5649 [details] patch As mentioned in #webkit, without the khtml:: prefix it doesn't compile, not sure why. Layout tests run fine. All other issues should be addressed in this patch.
Darin Adler
Comment 4 2006-01-13 22:53:52 PST
Comment on attachment 5649 [details] patch r=me
Note You need to log in before you can comment on or make changes to this bug.