Bug 29217

Summary: document.caretRangeFromPoint should take the scroll offset into account before hit testing
Product: WebKit Reporter: Xiaomei Ji <xji>
Component: DOMAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: annevk, jshin, mjs, sam, xji
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Description Flags
patch mjs: review+

Description Xiaomei Ji 2009-09-11 18:16:51 PDT
Created attachment 39496 [details]

There are 3 issues:

1. I think the offset in the returned range should be the character/caret
offset relative to the whole element, not relative to the viewpoint.
Please see the attached file caretRangeFromPointWithoutScroll.html for detailed

2. If I pass in event.clientX + document.body.scrollLeft and 
event.clientY + document.body.scrollTop as the x-axis and y-axis to
caretRangeFromPoint(), there is a possibility that the range is null which
should not. 
Please see attached file caretRangeFromPointWithScroll.html for detailed

3. In caretRangeFromPoint(), there is the following code:

    Node* node = result.innerNode();
    if (!node)
        return 0;

Since we do not check whether the 'node' is a Text related node, there is a
possibility that a range is created even when the mouse is not pointed in the
text area.

Please open caretRangeFromPointInNonTextNode.html,
click mouse right before the first word "type", in the range created by
caretRangeFromPoint(), the node is the Text node, the start/end offset is 0. 

click mouse several characters ahead of word "type" (the empty space in the
left of "type"), although the 'node' is a HTMLParagraphElement, but it has the
same visible position as the above, and the range created is the same as above.
Is this the correct behavior?

I think my assumption is that this function converts mouse position to the
character position within an element, so a null range should be returned if
click the empty spaces ahead of word "type".
Comment 1 Xiaomei Ji 2009-09-11 18:17:32 PDT
Created attachment 39497 [details]
Comment 2 Xiaomei Ji 2009-09-11 18:18:55 PDT
Created attachment 39498 [details]
Comment 3 Sam Weinig 2009-09-12 01:06:04 PDT
(In reply to comment #0)
> Created an attachment (id=39496) [details]
> caretRangeFromPointWithoutScroll.html
> There are 3 issues:

You should really file three bugs if you think there are three issues, one for each of these.  I am going to repurpose this bug to represent the first two, since I believe they represent the same issue, that we don't take the scroll offset into account before hit testing (which is the behavior of our elementFromPoint implementation, on which this API is modeled.  Our elementFromPoint seems to be /wrong/' in this regard [bug 29219]).
Comment 4 Sam Weinig 2009-09-12 01:10:04 PDT
Created attachment 39507 [details]
Comment 5 Maciej Stachowiak 2009-09-12 01:13:52 PDT
Comment on attachment 39507 [details]

Comment 6 Sam Weinig 2009-09-12 01:20:07 PDT
Fix landed in r48333.
Comment 7 Xiaomei Ji 2009-09-14 11:21:17 PDT
Thanks for the fixing. Filed another 2 bugs representing issue #2 and #3: