Bug 4904 - Bug with baseOffset and extentOffset in selections
Summary: Bug with baseOffset and extentOffset in selections
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Justin Garcia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-09-09 10:57 PDT by Sam Schillace
Modified: 2006-01-03 04:31 PST (History)
1 user (show)

See Also:


Attachments
test case (538 bytes, text/html)
2005-11-26 03:16 PST, Alexey Proskuryakov
no flags Details
patch (19.33 KB, patch)
2005-12-23 02:06 PST, Justin Garcia
no flags Details | Formatted Diff | Diff
patch-2 (19.33 KB, patch)
2005-12-23 02:41 PST, Justin Garcia
no flags Details | Formatted Diff | Diff
patch-3 (19.33 KB, patch)
2005-12-23 02:44 PST, Justin Garcia
no flags Details | Formatted Diff | Diff
patch-4 (19.12 KB, patch)
2005-12-23 02:46 PST, Justin Garcia
harrison: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Schillace 2005-09-09 10:57:33 PDT
This is actually two related bugs:

If a selection is formed with a double click, the baseOffset and extentOffset are the same, which is 
incorrect.

If the identical selection is made by dragging, the offsets are different, and correct, with one exception - if 
the selection is made by dragging right to left, the baseOffset will be greater than the extentOffset (e.g. 
the order of the offsets, and presumably the nodes as well, is reversed).
Comment 1 Alexey Proskuryakov 2005-11-26 03:16:12 PST
Created attachment 4806 [details]
test case
Comment 2 Alexey Proskuryakov 2005-11-26 03:18:18 PST
I am not sure about dragging from right to left, but the double-click behavior certainly looks like a bug.
Comment 3 Justin Garcia 2005-12-23 02:06:40 PST
Created attachment 5239 [details]
patch

Internally, our SelectionController uses the "base" to refer to the DOM
Position where the mouse was when the user began creating the selection, and
"extent" to refer to the DOM Position where the mouse was when the user
finalized the selection.  So the base will come before the extent for
selections created from left to right and the extent will come before the base
for selections created from right to left.  Given this definition of
base/extent, it makes sense that the base/extent would be equal for selections
created using a double or triple click, since the user started and finalized
the selection at the same DOM Position.

The SelectionController also has start() and end(), which refer to the DOM
Positions where the Selection actually starts/ends.

Unfortunately our JavaScript Selection object doesn't allow access to the
start/end!  We support the anchor/focus properties from Mozilla's Selection
object API, but anchor/focus just return base/extent.  In Mozilla's
documentation, anchor/focus are actually defined to be start/end (as start/end
were just defined above).  So this change changes anchor/focus, and leaves
base/extent alone.

This patch also adds getRangeAt from Mozilla's API, outlines the rest of the
API, and cleans up the JavaScript Selection object.

Next, I want to make the JS Selection object a wrapper for the internal
SelectionController object, instead of a wrapper for a KHTMLPart like it is
now.  Then I want to support the rest of the Mozilla Selection object API
(except the methods that only make sense for discontinuous selections).
Comment 4 Justin Garcia 2005-12-23 02:41:18 PST
Created attachment 5241 [details]
patch-2

A few tweaks.  This patch also adds an updateLayout call inside
VisiblePosition::init.	Instead of sprinkling the editing/selection code with
updateLayouts, I think we should just call it when whenever we're about to
create a VisiblePosition.  I should ask darin/harrison about this and then
remove all the updateLayout calls from the editing code.
Comment 5 Justin Garcia 2005-12-23 02:44:22 PST
Created attachment 5242 [details]
patch-3

attaching the correct patch
Comment 6 Justin Garcia 2005-12-23 02:46:02 PST
Created attachment 5243 [details]
patch-4

attaching the right patch this time, honest to god.
Comment 7 Justin Garcia 2005-12-23 14:00:00 PST
It looks like not all of the updateLayout calls in the editing code are used to ensure valid VisiblePositions, 
so not all updateLayouts could be removed after adding an updateLayout to VisiblePosition::init
Comment 8 David Harrison 2005-12-23 15:34:14 PST
Comment on attachment 5243 [details]
patch-4

r=me
Comment 9 Darin Adler 2005-12-27 11:08:00 PST
getRangeAt should return a PassRefPtr.
Comment 10 Justin Garcia 2006-01-03 04:31:12 PST
Made two changes before landing.  In FireFox, anchor/focus are the equal to the start/end of the 
selection, but reflect the direction in which the selection was made by the user.  That does not mean that 
they are base/extent, since the base/extent don't reflect expansion.  Also made darin's change.