Bug 27046 - Implement CSSOM DocumentView.caretRangeFromPoint
: Implement CSSOM DocumentView.caretRangeFromPoint
Status: RESOLVED FIXED
: WebKit
HTML DOM
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-07-07 15:49 PST by
Modified: 2009-09-11 17:44 PST (History)


Attachments
WIP patch (not ready for review) (3.02 KB, patch)
2009-07-10 22:43 PST, Sam Weinig
no flags Review Patch | Details | Formatted Diff | Diff
helpful test (862 bytes, text/html)
2009-07-10 22:46 PST, Sam Weinig
no flags Details
Little more work. (14.60 KB, patch)
2009-07-13 21:07 PST, Sam Weinig
no flags Review Patch | Details | Formatted Diff | Diff
patch w/ layout test (7.34 KB, patch)
2009-07-17 10:23 PST, Xiaomei Ji
no flags Review Patch | Details | Formatted Diff | Diff
caretRangeFromPointWithoutScroll.html (2.01 KB, text/html)
2009-09-11 16:21 PST, Xiaomei Ji
no flags Details
caretRangeFromPointWithScroll.html (1.57 KB, text/html)
2009-09-11 16:23 PST, Xiaomei Ji
no flags Details
caretRangeFromPointInNonTextNode.html (1.13 KB, text/html)
2009-09-11 17:05 PST, Xiaomei Ji
no flags Details


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-07-07 15:49:01 PST
Implement CSSOM DocumentView.caretRangeFromPoint

Right now it's not possible to find out what word is under the mouse.  Adding this new feature from CSSOM would solve this (and other related) issue(s).

interface DocumentView {
  readonly attribute AbstractView defaultView;
  Element elementFromPoint(in float x, in float y);
  Range caretRangeFromPoint(in float x, in float y);
};
------- Comment #1 From 2009-07-07 15:49:57 PST -------
http://dev.w3.org/csswg/cssom-view/#documentview-caretrangefrompoint is the spec URL btw.
------- Comment #2 From 2009-07-10 22:43:30 PST -------
Created an attachment (id=32610) [details]
WIP patch (not ready for review)
------- Comment #3 From 2009-07-10 22:46:39 PST -------
Created an attachment (id=32611) [details]
helpful test

Attaching a little helpful test I made when writing this code.  With the patch applied, when you click, the position where you click extended one character forward is selected as an indication.
------- Comment #4 From 2009-07-13 21:07:22 PST -------
Created an attachment (id=32694) [details]
Little more work.
------- Comment #5 From 2009-07-17 10:23:05 PST -------
Created an attachment (id=32952) [details]
patch w/ layout test

Hi Sam,

I did not add myself into the cc list, and I am not aware that you are doing the implementation.
I am uploading my patch just for reference. 

Thanks,
Xiaomei
------- Comment #6 From 2009-07-17 10:59:47 PST -------
(In reply to comment #4)
> Created an attachment (id=32694) [details] [details]
> Little more work.

Hi Sam,

Thanks for implementing this!

I have several questions regarding your patch:
1. The CSSOM spec says that: "If ..... no insertion point indicator would have been inserted, or editing is not supported, the method must return null". I do not quite understand why it returns NULL if "editing is not supported". So, I changed the implementation to "return NULL if the node type is not text related node type". For other node types, range::create(Document*, Position, Position) might raise error code and hit assertion failure (for example, "input" element). Or rangeCompliantEquivalent takes care of that? 

2. When calculating the coordinate relative to document, you are taking zoom factor into consideration. Should it also take document scrolling into consideration as well? Since the spec says (x, y) is coordinates in viewport.
Or maybe we can use coordinate relative to document and let JS do  the computation.

3. Could you please add some comments in caretRangeFromPoint() declaration in Document.h?
For a new comer like me, comments help him/her to learn the code, especially what/relative to which a point is.

Thanks,
Xiaomei
------- Comment #7 From 2009-07-17 12:36:50 PST -------
(In reply to comment #6)
> (In reply to comment #4)
> > Created an attachment (id=32694) [details] [details] [details]
> > Little more work.
> 
> Hi Sam,
> 
> Thanks for implementing this!
> 
> I have several questions regarding your patch:
> 1. The CSSOM spec says that: "If ..... no insertion point indicator would have
> been inserted, or editing is not supported, the method must return null". I do
> not quite understand why it returns NULL if "editing is not supported". So, I
> changed the implementation to "return NULL if the node type is not text related
> node type". For other node types, range::create(Document*, Position, Position)
> might raise error code and hit assertion failure (for example, "input"
> element). Or rangeCompliantEquivalent takes care of that? 

I don't think limiting to just text related nodes is the right way to go since that removes are replaced elements as well.  I used a combination of checking for shadow tree content (input elements, etc.) and using rangeComplientEquivalent.  The spec is not all that clear, so we should probably confer with the editor on the intent.

> 
> 2. When calculating the coordinate relative to document, you are taking zoom
> factor into consideration. Should it also take document scrolling into
> consideration as well? Since the spec says (x, y) is coordinates in viewport.
> Or maybe we can use coordinate relative to document and let JS do  the
> computation.

All computation should be done relative to the viewport, so I think scrolling is already accounted for, though adding tests is the way to be sure.
------- Comment #8 From 2009-07-17 12:50:36 PST -------
> > 2. When calculating the coordinate relative to document, you are taking zoom
> > factor into consideration. Should it also take document scrolling into
> > consideration as well? Since the spec says (x, y) is coordinates in viewport.
> > Or maybe we can use coordinate relative to document and let JS do  the
> > computation.
> 
> All computation should be done relative to the viewport, so I think scrolling
> is already accounted for, though adding tests is the way to be sure.

If HitTest is done relative to viewport when the document is scrolled, the computed range offset is shifted too (relative to viewport, not relative to the element within the document).
I added the adjustment when I ran test. But I do not know whether zoom factor or other part in your patch takes care of that. And you are right, adding test is the way to be sure.

Thanks!
Xiaomei
------- Comment #9 From 2009-07-27 23:55:51 PST -------
"editing is not supported" was there because the use case for this method was editors. However, it seems better to remove that since finding a good definition for it seems hard and applications want to check something like that a better higher level as well.

The intent of the specification is that if you have designMode set to "on" you'd get a range for where the cursor appears when you click or null if no cursor appears. Is that clear enough?
------- Comment #10 From 2009-07-28 00:32:07 PST -------
I agree, "editing is not supported" should be removed.
------- Comment #11 From 2009-09-08 15:03:18 PST -------
Landed in r48188.
------- Comment #12 From 2009-09-11 16:21:43 PST -------
Created an attachment (id=39487) [details]
caretRangeFromPointWithoutScroll.html
------- Comment #13 From 2009-09-11 16:23:28 PST -------
Created an attachment (id=39488) [details]
caretRangeFromPointWithScroll.html
------- Comment #14 From 2009-09-11 16:24:34 PST -------
Hi Sam,

Thanks for implementing this!

It worked pretty well except the 2 issues I list below:
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 explanation.

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 explanation.

Please let me know if I mis-understood anything, or the code needs a bit tweak.

Thanks,
Xiaomei
------- Comment #15 From 2009-09-11 17:05:37 PST -------
Created an attachment (id=39493) [details]
caretRangeFromPointInNonTextNode.html
------- Comment #16 From 2009-09-11 17:18:40 PST -------
Another question:

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".

Thanks,
Xiaomei
------- Comment #17 From 2009-09-11 17:44:37 PST -------
This bug is now closed.  Please file any bugs in the implementation in a separate bug.