Closed Bug 681387 Opened 13 years ago Closed 13 years ago

caretPositionFromPoint seems to return different offset as what the selection offset is (backout bug 654352 )

Categories

(Core :: DOM: Core & HTML, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Tracking Status
firefox9 + fixed

People

(Reporter: martijn.martijn, Assigned: smaug)

References

Details

(Keywords: testcase, verified-aurora, verified-beta, Whiteboard: [qa!])

Attachments

(4 files, 1 obsolete file)

Attached file 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).
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
Attached file 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.
Brad, are you perhaps looking at this?
I guess we need to back out bug 654352 ASAP, especially from Aurora.
Ehsan, can you look into this? It's somewhat editor-ish :-)
Assignee: nobody → ehsan
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.
we backed out the front end patch that's using it. IMO the silent updates are more important
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.
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).
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.
(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.
(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... :-)
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.
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.
One way to reconcile this issue is to make Selection instantiatable anywhere and let caretPositionFromPoint return a collapsed selection.
Attached patch backout from trunk (obsolete) — Splinter Review
uploaded to try.

I'll update the patch for aurora.
Assignee: ehsan → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #565594 - Flags: review?(ehsan)
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...
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.
Attachment #565594 - Attachment is obsolete: true
Attachment #565594 - Flags: review?(ehsan)
Attachment #565600 - Flags: review?(ehsan)
Attachment #565601 - Flags: review?(ehsan)
(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.
Attachment #565601 - Flags: review?(ehsan) → review+
Attachment #565600 - Flags: review?(ehsan) → review+
Keywords: dev-doc-needed
Attachment #565600 - Flags: approval-mozilla-aurora?
Attachment #565600 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/c3e00a0f16cc
https://hg.mozilla.org/mozilla-central/rev/b4da2d439cbc
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Summary: caretPositionFromPoint seems to return different offset as what the selection offset is → caretPositionFromPoint seems to return different offset as what the selection offset is (backout bug 654352 )
No docs needed, as 654352 hadn't been written up yet. Will track bug 654352 to know when to do the doc for this.
Keywords: dev-doc-needed
Whiteboard: [qa+]
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.
Status: RESOLVED → VERIFIED
Whiteboard: [qa+] → [qa!]
Component: DOM: Other → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: