Last Comment Bug 681387 - caretPositionFromPoint seems to return different offset as what the selection offset is (backout bug 654352 )
: caretPositionFromPoint seems to return different offset as what the selection...
Status: VERIFIED FIXED
[qa!]
: testcase, verified-aurora, verified-beta
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Windows 7
: -- normal with 1 vote (vote)
: ---
Assigned To: Olli Pettay [:smaug] (TPAC)
:
Mentors:
Depends on:
Blocks: 654352 667243
  Show dependency treegraph
 
Reported: 2011-08-23 10:45 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2013-04-04 13:52 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
testcase (1.67 KB, text/html)
2011-08-23 10:45 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
testcase2 (2.25 KB, text/html)
2011-08-24 01:35 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
backout from trunk (14.29 KB, patch)
2011-10-07 11:20 PDT, Olli Pettay [:smaug] (TPAC)
no flags Details | Diff | Splinter Review
Backout patch for aurora (19.61 KB, patch)
2011-10-07 11:52 PDT, Olli Pettay [:smaug] (TPAC)
ehsan: review+
blassey.bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
backout from trunk (19.61 KB, patch)
2011-10-07 11:53 PDT, Olli Pettay [:smaug] (TPAC)
ehsan: review+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2011-08-23 10:45:01 PDT
Created attachment 555133 [details]
testcase

Afaik, the document.caretPositionFromPoint method should return the caret position as to where the caret would be positioned, when you would click/tap on that point in the document.
The resulting offsetNode and offset properties would be equal to the collapsed selection object's focusNode and focusOffset (and in the textfield case, the selectionEnd).
(let me know if this assumption is incorrect).

The testcase shows the results of these methods/properties, when you click somewhere in the black bordered box.

I can't really make sense of the offset value of document.caretPositionFromPoint.
It always seems to be higher than the selection's focusOffset value (at least 1 or 2 points to high).
Comment 1 Olli Pettay [:smaug] (TPAC) 2011-08-23 11:08:12 PDT
Oh, this is very valid.

/me kicks himself for not requiring more testing for bug 654352

We must not release FF9 with this bug unfixed.
Either need to fix this or back out bug 654352 from next aurora
Comment 2 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-08-24 01:35:08 PDT
Created attachment 555343 [details]
testcase2

Inside an iframe, also returns different results, steps to reproduce:
- Mouseover the "selection removeAllRanges onmouseover" button
- Click somewhere inside the iframe

Notice how the selection object returns null and 0, while the caretPositionFromPoint method returns the html element and 0.

Btw, then clicking on the 'setcaret at last caretpositionfrompoint thingy' button results in a NS_ERROR_FAILURE, I think that is bug 454788.
Comment 3 Olli Pettay [:smaug] (TPAC) 2011-09-19 22:47:18 PDT
Brad, are you perhaps looking at this?
Comment 4 Olli Pettay [:smaug] (TPAC) 2011-10-01 01:22:00 PDT
I guess we need to back out bug 654352 ASAP, especially from Aurora.
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-01 02:13:34 PDT
Ehsan, can you look into this? It's somewhat editor-ish :-)
Comment 6 :Ehsan Akhgari 2011-10-06 15:31:25 PDT
How important is this?  I know the code responsible, so I can work on this, but I have some silent update stuff on my plate which is really high priority.
Comment 7 Brad Lassey [:blassey] (use needinfo?) 2011-10-06 15:34:00 PDT
we backed out the front end patch that's using it. IMO the silent updates are more important
Comment 8 :Ehsan Akhgari 2011-10-06 16:52:05 PDT
OK, I took a quick look at things.  Basically we have three problems here:

1. The spec is broken.  For example, it refers to "caret range" without defining it in the CaretPosition object.
2. The implementation is broken.  It doesn't follow the spec in any way, and fixing it basically maps to rewriting it from the scratch.  For example, it completely ignores the fact that we don't insert carets everywhere.
3. I'm not sure if this is a useful web API in the first place.  Can somebody please let me know why we implemented this in the first place?  While I can see use cases for a hit-testing API, this is not that API, and I'm not convinced that we should implement it at all.
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-06 17:04:33 PDT
Well, we should back this API out for starters.

I think the mobile folks want to be able to draw a marker when the user does some kind of selection action, where the caret would be if the user clicked there, except without moving the caret.

Other use cases are about finding which character in a text node is under the mouse cursor (or some other point).
Comment 10 Alfonso Martinez 2011-10-06 23:58:05 PDT
I want to use this API to allow direct upload of files in CKEditor as seen in this video: http://www.youtube.com/watch?v=DVInjn51VYw

Currently I'm using in Firefox the rangeParent and rangeOffset of the drop event so it's working fine to insert the new element (image or link) at the drop position, but it will be nicer to use a single API for all the browsers.

Other drop related situations (but of course they can be solved in Firefox with those properties) are cleaning up of dropped HTML to remove all or just some tags, in which case we need to avoid the default insertion by the browser and find the correct spot to perform the insertion.
Comment 11 Olli Pettay [:smaug] (TPAC) 2011-10-07 03:24:31 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #8)
> OK, I took a quick look at things.  Basically we have three problems here:
> 
> 1. The spec is broken.  For example, it refers to "caret range" without
> defining it in the CaretPosition object.
Because caret range was actually removed from CaretPosition.
a range doesn't work with form controls.


> For example, it
> completely ignores the fact that we don't insert carets everywhere.
What has this to do with the API?


> 3. I'm not sure if this is a useful web API in the first place. 
Sure it is useful to know where caret would be placed if user clicked somewhere.
That is why Gecko has had rangeParent and rangeOffset in UIEvents for ages.
Comment 12 :Ehsan Akhgari 2011-10-07 09:21:40 PDT
(In reply to Olli Pettay [:smaug] from comment #11)
> (In reply to Ehsan Akhgari [:ehsan] from comment #8)
> > OK, I took a quick look at things.  Basically we have three problems here:
> > 
> > 1. The spec is broken.  For example, it refers to "caret range" without
> > defining it in the CaretPosition object.
> Because caret range was actually removed from CaretPosition.
> a range doesn't work with form controls.

Yes, but the algorithm for the API as appears in the spec still refers to the caret range.

> > For example, it
> > completely ignores the fact that we don't insert carets everywhere.
> What has this to do with the API?

Not sure what you mean.  My point was that the implementation does not correctly implement the specified API.

> > 3. I'm not sure if this is a useful web API in the first place. 
> Sure it is useful to know where caret would be placed if user clicked
> somewhere.
> That is why Gecko has had rangeParent and rangeOffset in UIEvents for ages.

I mainly think there are two things wrong with this API:

a) It only specifies what should happen with text controls.  It doesn't really talk about what "the position where the text insertion point indicator would have been inserted" means, exactly.  Given the current spec, implementations are free to return whatever they want.  Specifically, in order to specify something meaningful, we need a definition of the concept of caret (today defined by all major UAs as collapsed selections in some circumstances), and we also need to specify how the selection/caret position should be normalized when for example clicking between two inline elements.
b) It uses a very weird object to denote caret position.  The way that CaretPosition is defined, it is fundamentally different to both the Selection object, and the selectionStart/selectionEnd properties.

Therefore, I think we should consider backing out this API.  In fact, I would go further and request that it should be removed from the CSSOM spec in its current form, but I haven't really interacted with the people writing that spec before... :-)
Comment 13 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-10-07 09:44:21 PDT
Yes, that is spec fodder.
For Mozilla, I believe it is quite clear what caretPositionFromPoint should return, and that is the selection as if the user has clicked at that specific point.
Comment 14 Olli Pettay [:smaug] (TPAC) 2011-10-07 09:52:15 PDT
As I said on IRC, I don't understand what is wrong with the API. Or it is no worse than elementFromPoint.
The idea is to return the position of possible caret if user clicked on x,y.

But, I'll post a backout patch anyway. We shouldn't ship broken implementation of the API.
Comment 15 Ryosuke Niwa 2011-10-07 11:05:46 PDT
One way to reconcile this issue is to make Selection instantiatable anywhere and let caretPositionFromPoint return a collapsed selection.
Comment 16 Olli Pettay [:smaug] (TPAC) 2011-10-07 11:20:03 PDT
Created attachment 565594 [details] [diff] [review]
backout from trunk

uploaded to try.

I'll update the patch for aurora.
Comment 17 :Ehsan Akhgari 2011-10-07 11:30:04 PDT
OK, imaging this markup.

<p style="width:1000px;"><em><span>test</span></em></p>

And let's say that the API is called with coordinates corresponding to the blank section at the right hand side of "test" (but inside the paragraph).  What should the function return?

Where the selection goes is an implementation detail at least for now, and I'm pretty sure that not all UAs agree on what should happen here...
Comment 18 rniwa 2011-10-07 11:36:42 PDT
The fact UA diverges in its behavior here is fine since it still addresses the primary use case. In fact, this API is needed precisely because hit-testing is UA-dependent and will be really hard to match each other if possible at all.

What bothers me is your another point that we're adding new interface to communicate this information, and isn't sufficient to determine where the caret is rendered. e.g. in a mixed bidi content, it's non-trivial to determine whether the caret is rendered because DOM position you get out of CaretPosition interface as defined now doesn't tell you to which bidi level the caret belongs. Also, some UAs may use split-caret or other mechanism at bidi-boundaries.
Comment 19 Olli Pettay [:smaug] (TPAC) 2011-10-07 11:52:27 PDT
Created attachment 565600 [details] [diff] [review]
Backout patch for aurora
Comment 20 Olli Pettay [:smaug] (TPAC) 2011-10-07 11:53:08 PDT
Created attachment 565601 [details] [diff] [review]
backout from trunk
Comment 21 :Ehsan Akhgari 2011-10-07 12:57:51 PDT
(In reply to rniwa from comment #18)
> The fact UA diverges in its behavior here is fine since it still addresses
> the primary use case. In fact, this API is needed precisely because
> hit-testing is UA-dependent and will be really hard to match each other if
> possible at all.

While hit-testing being different here is also an issue, I was mostly pointing at the differences between the way the UAs would normalize the selection (if they do it at all).  In my example, do you expect the selection to be on the span element, the em or the div element?

> What bothers me is your another point that we're adding new interface to
> communicate this information, and isn't sufficient to determine where the
> caret is rendered. e.g. in a mixed bidi content, it's non-trivial to
> determine whether the caret is rendered because DOM position you get out of
> CaretPosition interface as defined now doesn't tell you to which bidi level
> the caret belongs. Also, some UAs may use split-caret or other mechanism at
> bidi-boundaries.

Yes, that is still a concern.
Comment 23 Eric Shepherd [:sheppy] 2011-10-10 12:16:06 PDT
No docs needed, as 654352 hadn't been written up yet. Will track bug 654352 to know when to do the doc for this.
Comment 24 Ioana (away) 2011-12-08 04:42:35 PST
Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:9.0) Gecko/20100101 Firefox/9.0 (20111206234556)
Mozilla/5.0 (Windows NT 6.1; rv:10.0a2) Gecko/20111207 Firefox/10.0a2
Mozilla/5.0 (Windows NT 6.1; rv:11.0a1) Gecko/20111207 Firefox/11.0a1

The "document.caretPositionFromPoint is not a function" error is displayed in the error console when trying to use the test cases attached to this bug, which means bug 654352 has been backed out.

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