Last Comment Bug 654352 - document.caretPositionFromPoint API
: document.caretPositionFromPoint API
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla20
Assigned To: Scott Johnson (:jwir3)
:
Mentors:
: 669816 (view as bug list)
Depends on: 826069 681387 824965 825499 826060
Blocks: 668228 667243
  Show dependency treegraph
 
Reported: 2011-05-02 18:33 PDT by liucougar
Modified: 2013-04-04 13:52 PDT (History)
29 users (show)
graeme.mccutcheon: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
patch (5.13 KB, patch)
2011-06-27 10:23 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
patch (3.71 KB, patch)
2011-08-05 18:46 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
patch (5.85 KB, patch)
2011-08-05 23:47 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
patch to add DOMClassInfo bits (2.24 KB, patch)
2011-08-06 10:06 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
patch (12.16 KB, patch)
2011-08-06 13:30 PDT, Brad Lassey [:blassey] (use needinfo?)
roc: review+
Details | Diff | Review
patch (12.50 KB, patch)
2011-08-07 20:24 PDT, Brad Lassey [:blassey] (use needinfo?)
bugs: review+
Details | Diff | Review
patch, ready to land (12.43 KB, patch)
2011-08-08 08:23 PDT, Brad Lassey [:blassey] (use needinfo?)
blassey.bugs: review+
Details | Diff | Review
test patch, WIP (1.94 KB, patch)
2011-08-08 08:26 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
patch w/test and anon content handling (14.81 KB, patch)
2011-08-12 06:38 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
patch (15.34 KB, patch)
2011-08-15 22:36 PDT, Brad Lassey [:blassey] (use needinfo?)
bugs: review-
Details | Diff | Review
patch (15.67 KB, patch)
2011-08-17 06:55 PDT, Brad Lassey [:blassey] (use needinfo?)
bugs: review-
Details | Diff | Review
patch (16.19 KB, patch)
2011-08-18 08:38 PDT, Brad Lassey [:blassey] (use needinfo?)
bugs: review+
mark.finkle: approval‑mozilla‑aurora-
Details | Diff | Review
patch to moz prefix (2.42 KB, patch)
2011-08-19 10:32 PDT, Brad Lassey [:blassey] (use needinfo?)
bugs: review+
Details | Diff | Review
b654352 (v2) (16.24 KB, patch)
2012-12-18 10:09 PST, Scott Johnson (:jwir3)
no flags Details | Diff | Review
b654352 (v3, now with more tests) (18.41 KB, patch)
2012-12-18 15:14 PST, Scott Johnson (:jwir3)
blassey.bugs: review+
Details | Diff | Review
b654352 (v4) (16.94 KB, patch)
2012-12-20 13:34 PST, Scott Johnson (:jwir3)
Ms2ger: review+
Details | Diff | Review

Description liucougar 2011-05-02 18:33:38 PDT
according to:
http://www.w3.org/TR/cssom-view/#dom-documentview-caretrangefrompoint

The caretRangeFromPoint(x, y) method, when invoked, must return an empty text range for the position where a text insertion point indicator would have been inserted if editing was enabled and hit testing was performed at the coordinates x,y in the viewport. If either argument is negative, x is greater than the viewport width excluding the size of a rendered scroll bar (if any), y is greather than the viewport height excluding the size of a rendered scroll bar (if any), or no insertion point indicator would have been inserted, the method must return null. [DOM2TR]

this is the standard way of mapping between a client coordinate to a range, which can replace the rangeParent/rangeOffset properties on mouse events (which are not standard, and only implemented in Firefox, and limited to mouse events)
Comment 1 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-05-03 01:56:20 PDT
(In reply to comment #0)
> according to:
> http://www.w3.org/TR/cssom-view/#dom-documentview-caretrangefrompoint

That document is old. The valid one is 
http://dev.w3.org/csswg/cssom-view/#extensions-to-the-document-interface
which doesn't have caretRangeFromPoint.
caretRangeFromPoint has some obvious problems for example with 
replaced elements, such as <input> and <textarea>.
Comment 2 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-05-03 01:57:12 PDT
Changing the summary. We can use this bug for caretPositionFromPoint.
Comment 3 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-05-03 02:00:25 PDT
...but how caretPositionFromPoint is specified needs to be still
reviewed properly before implementing it.
Comment 4 liucougar 2011-05-03 12:40:48 PDT
does it make sense to just expose the underlying logic for generating rangeParent/rangeOffset on mouse events? (adjust it for any inconsistency with regards to current draft)

IMHO, that something is not properly specified/reviewed does not mean it can't be implemented (if that's the case, no new feature would have been introduced)
Comment 5 Jonas Sicking (:sicking) PTO Until July 5th 2011-05-04 10:59:00 PDT
liucougar: the most important part of reviewing a spec is figuring out how we *want* things to work. Obviously we can't implement an API without figuring that out.

Another important aspect is that I think we should moz-prefix things.
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-04 16:52:41 PDT
I think CaretPosition needs an extra flag to indicate "hint left" or "hint right", the equivalent of our ContentOffsets::associateWithNext. Other than that, the caretPositionFromPoint spec looks good to me.
Comment 7 liucougar 2011-05-04 17:31:19 PDT
ContentOffsets::associateWithNext sounds useful in the CaretPosition

I may be able to give it a shot at coming up with a patch to implement this. could anyone provide some hints on where to start?
Comment 8 liucougar 2011-05-04 17:35:38 PDT
looks like nsIFrame::GetContentOffsetsFromPoint can do the heavy lifting, but it has the following comments: 
/**
 * This function calculates the content offsets for selection relative to
 * a point.  Note that this should generally only be callled on the event
 * frame associated with an event because this function does not account
 * for frame lists other than the primary one.
 * @param aPoint point relative to this frame
 */

the "generally only be callled on the event frame" part concerns me: document.caretPositionFromPoint can be called from places other than event handlers.
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-04 19:07:55 PDT
That's fine. The important thing is that you first use the point to identify the frame that would handle a mouse event at that point, i.e. by calling nsLayoutUtils::GetFrameForPoint. Then you call GetEventCoordinatesRelativeTo with that frame, then you call GetContentOffsetsFromPoint on that frame.
Comment 10 Brad Lassey [:blassey] (use needinfo?) 2011-06-27 10:23:23 PDT
Created attachment 542194 [details] [diff] [review]
patch
Comment 11 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-27 14:40:12 PDT
Comment on attachment 542194 [details] [diff] [review]
patch

Review of attachment 542194 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsDocument.cpp
@@ +8402,4 @@
>    return NS_OK;
>  }
>  
> +class nsDOMCaretPosition : public nsIDOMCaretPosition

I think we need cycle collection support here, since you're holding a strong reference to a node. Imagine what happens if a script stores a CaretPosition as an expando property of the DOM node, for example.

@@ +8435,5 @@
> +{
> +  nsCOMPtr<nsIPresShell> shell = GetShell();
> +  nsIFrame* rootFrame = shell->GetRootFrame();
> +  nsPoint widgetOffset;
> +  nsIWidget* widget = rootFrame->GetView()->GetNearestWidget(&widgetOffset);

Why are we going through views and widgets here?

I think you can just find the frame for the point using nsLayoutUtils::GetFrameForPoint like ElementFromPointHelper does, then you can call nsIFrame::GetContentOffsetsFromPoint on that frame.

There is one issue I hadn't thought of, which is that document.caretPositionFromPoint should always return a node in that document (similar to what ElementFromPointHelper does). So when you use this from chrome, you'll need to make sure you first find the node at that position from *any* document, and then call caretPositionFromPoint on its document. Make sense?

::: dom/interfaces/core/nsIDOMDocument.idl
@@ +52,5 @@
> +[scriptable, uuid(cf5ad6cf-6f49-4ca7-9b50-590d7bb27a13)]
> +interface nsIDOMCaretPosition : nsISupports {
> +  readonly attribute nsIDOMNode offsetNode;
> +  readonly attribute unsigned long offset;
> +};

I think this deserves its own file. nsIDOMDocument is a busy file already!
Comment 12 Brad Lassey [:blassey] (use needinfo?) 2011-06-29 22:39:56 PDT
(In reply to comment #11)
> Comment on attachment 542194 [details] [diff] [review] [review]
> patch
> 
> Review of attachment 542194 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: content/base/src/nsDocument.cpp
> @@ +8402,4 @@
> >    return NS_OK;
> >  }
> >  
> > +class nsDOMCaretPosition : public nsIDOMCaretPosition
> 
> I think we need cycle collection support here, since you're holding a strong
> reference to a node. Imagine what happens if a script stores a CaretPosition
> as an expando property of the DOM node, for example.
Is there any documentation for implementing cycle collecting support?
Comment 13 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-29 22:42:34 PDT
I just cargo-cult it from another similar object.
Comment 14 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-06-30 04:35:31 PDT
That is the easiest way, but yes, there is even some documentation.
https://developer.mozilla.org/en/Interfacing_with_the_XPCOM_cycle_collector
Comment 15 Mark Finkle (:mfinkle) (use needinfo?) 2011-07-21 08:22:46 PDT
*** Bug 669816 has been marked as a duplicate of this bug. ***
Comment 16 Mark Finkle (:mfinkle) (use needinfo?) 2011-07-21 08:24:18 PDT
This API is really needed for Fennec's text selection features
Comment 17 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-07-21 08:57:09 PDT
Really? I would have thought that using mouse events' rangeParent/offset would be enough for Fennec.

This is still very useful feature.
Comment 18 Mark Finkle (:mfinkle) (use needinfo?) 2011-07-21 10:56:37 PDT
(In reply to comment #17)
> Really? I would have thought that using mouse events' rangeParent/offset
> would be enough for Fennec.

We do this now and the mouse events cause other actions to happen, when positioned over a link or textbox, for example. We have hacks in place to try and stop the effects.
Comment 19 Brad Lassey [:blassey] (use needinfo?) 2011-07-21 10:58:24 PDT
(In reply to comment #16)
> This API is really needed for Fennec's text selection features

text selection landed in 7, so it should track 7
Comment 20 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-07-21 11:04:43 PDT
Comment on attachment 542194 [details] [diff] [review]
patch

nsDOMCaretPosition needs to be cycle collectable, otherwise you may leak
mNode and everything it keeps alive.

Coding style is
if (expr) {
  stmt
}
Comment 21 Asa Dotzler [:asa] 2011-07-21 14:55:09 PDT
trackingfirefox7 because this could have implications for more than just Fennec.
Comment 22 Johnny Stenback (:jst, jst@mozilla.com) 2011-07-21 15:00:13 PDT
Comment on attachment 542194 [details] [diff] [review]
patch

@@ -362,4 +368,6 @@ interface nsIDOMDocument : nsIDOMNode
    */
   void mozSetImageElement(in DOMString aImageElementId,
                           in nsIDOMElement aImageElement);
+
+  nsIDOMCaretPosition caretPositionFromPoint(in float x, in float y);

This will need an IID bump on trunk, and a separate interface on branches, if we'll be taking this on branches...
Comment 23 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-07-21 15:01:55 PDT
Also, the patch is actually implementing something else than what the spec says ;)
Comment 24 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-07-28 08:33:22 PDT
I added a patch in bug 669816, that basicaly mimimicks mouse-drag selection as in desktop Firefox. It seems to work well. Maybe something like that could be used, instead?
Comment 25 Yves Goergen 2011-08-05 00:43:08 PDT
I don't understand the formulation here, but will this request allow me to somehow translate between X/Y coordinates on the page to a character index of an input/textarea element, like that of the current cursor/caret position? I mean it the way to display a dropdown list at the cursor location, or (the other direction) to determine which word in a textarea is hovered by the mouse. If that's something else, is there another bug for it (this is the only one I've found) or can I open a new one for it?
Comment 26 Brad Lassey [:blassey] (use needinfo?) 2011-08-05 18:46:57 PDT
Created attachment 551213 [details] [diff] [review]
patch
Comment 27 Brad Lassey [:blassey] (use needinfo?) 2011-08-05 23:47:19 PDT
Created attachment 551234 [details] [diff] [review]
patch

forgot to add the idl file
Comment 28 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-08-06 05:34:43 PDT
(In reply to Olli Pettay [:smaug] from comment #23)
> Also, the patch is actually implementing something else than what the spec
> says ;)

Can you elaborate on that?
Comment 29 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-06 05:54:39 PDT
The first patch was changing selection, and was doing things like HandleClick.
That stuff has been removed from the new patch, which looks almost
ok to me. (There should be tests, and I believe the patch is missing
stuff which should be added to nsDOMClassInfo so that the class gets right nsIClassInfo )
Comment 30 Brad Lassey [:blassey] (use needinfo?) 2011-08-06 10:06:44 PDT
Created attachment 551262 [details] [diff] [review]
patch to add DOMClassInfo bits

here's a follow up patch to add the DOMClassInfo stuff you mentioned
Comment 31 :Ms2ger 2011-08-06 10:40:41 PDT
Comment on attachment 551234 [details] [diff] [review]
patch

Some nit, and one serious, probably-exploitable bug.

>--- a/content/base/src/nsDocument.cpp
>+++ b/content/base/src/nsDocument.cpp
>+class nsDOMCaretPosition : public nsIDOMCaretPosition

I would kind of like to give this its own file as well... There's a lot of junk in nsDocument.cpp already.

>+  NS_IMETHOD GetOffsetNode(nsIDOMNode **aOffsetNode)

nsIDOMNode** aOffsetNode

>+    *aOffsetNode = mNode;

nsCOMPtr<nsIDOMNode> node = mNode;
node.forget(aOffsetNode);

(Always addref outparams.) <-- Serious bug

>+  NS_IMETHOD GetOffset(PRUint32 *aOffset)

PRUint32* aOffset

>+  nsDOMCaretPosition(nsIDOMNode* aNode, PRUint32 aOffset) :
>+    mNode(aNode), mOffset(aOffset) {}

  nsDOMCaretPosition(nsIDOMNode* aNode, PRUint32 aOffset)
    : mNode(aNode), mOffset(aOffset) {}

>+  nsCOMPtr<nsIDOMNode> mNode;

Why is this public?

>+  /* additional members */

This comment can go, IMO

>+  PRUint32 mOffset;
>+

As well as this empty line.

>+nsDocument::CaretPositionFromPoint(float aX, float aY, nsIDOMCaretPosition **aCaretPos)

nsIDOMCaretPosition** aCaretPos

>+{
>+

Here too

>+  if (aCaretPos) {

Anybody who passes in null here deserves to crash.

>--- /dev/null
>+++ b/dom/interfaces/core/nsIDOMCaretPosition.idl
>@@ -0,0 +1,44 @@
>+ * The Initial Developer of the Original Code is
>+ * Mozilla Corporation.

"the Mozilla Foundation."

http://weblogs.mozillazine.org/gerv/archives/2010/02/mpl_initial_developer_for_mozilla_employ.html
Comment 32 Brad Lassey [:blassey] (use needinfo?) 2011-08-06 13:30:47 PDT
Created attachment 551275 [details] [diff] [review]
patch
Comment 33 Brad Lassey [:blassey] (use needinfo?) 2011-08-06 13:34:47 PDT
> >+  if (aCaretPos) {
> 
> Anybody who passes in null here deserves to crash.
As I understand it, that crash would be exploitable from content by simply calling 
"doc.caretPositionFromPoint(0,0)" with nothing to receive the return value. To protect against this I added this to the beginning of the function:

  NS_ENSURE_ARG_POINTER(aCaretPos);
  *aCaretPos = nsnull;
Comment 34 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-06 13:39:13 PDT
content cannot call caretPositionFromPoint without "receiving" the return value.
Comment 35 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-08-07 17:39:07 PDT
Comment on attachment 551275 [details] [diff] [review]
patch

Review of attachment 551275 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with that fixed. I think Olli should review this too.

::: content/base/src/nsDocument.cpp
@@ +8413,5 @@
> +    return NS_OK;
> +  nsFrame::ContentOffsets offsets = ptFrame->GetContentOffsetsFromPoint(pt);
> +  nsCOMPtr<nsIDOMNode> node = do_QueryInterface(offsets.content);
> +
> +  *aCaretPos = new nsDOMCaretPosition(node, offsets.StartOffset());

I think you should use .offset here. Per the documention of ContentOffsets: "The primary offset is the expected position of the cursor calculated from a point".

::: dom/interfaces/core/nsIDOMDocument.idl
@@ +363,5 @@
>     */
>    void mozSetImageElement(in DOMString aImageElementId,
>                            in nsIDOMElement aImageElement);
> +
> +  nsIDOMCaretPosition caretPositionFromPoint(in float x, in float y);

IID rev.
Comment 36 Brad Lassey [:blassey] (use needinfo?) 2011-08-07 20:24:58 PDT
Created attachment 551384 [details] [diff] [review]
patch
Comment 37 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-08 03:21:03 PDT
Comment on attachment 551384 [details] [diff] [review]
patch


>+class nsDOMCaretPosition : public nsIDOMCaretPosition
>+{
>+public:
>+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
>+  NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsDOMCaretPosition, nsIDOMCaretPosition)
_AMBIGUOUS is probably not needed



>+  NS_IMETHOD GetOffsetNode(nsIDOMNode** aOffsetNode);
>+  NS_IMETHOD GetOffset(PRUint32* aOffset);
NS_DECL_NSIDOMCARETPOSITION

>+  nsDOMCaretPosition(nsIDOMNode* aNode, PRUint32 aOffset);
>+
>+private:
>+  nsCOMPtr<nsIDOMNode> mNode;
>+
>+protected:
>+  virtual ~nsDOMCaretPosition();
>+  PRUint32 mOffset;

Why mOffset is protected but mNode is private?
Both could be just protected, or private.



>+nsDocument::CaretPositionFromPoint(float aX, float aY, nsIDOMCaretPosition** aCaretPos)
>+{
>+  NS_ENSURE_ARG_POINTER(aCaretPos);
>+  *aCaretPos = nsnull;
>+  
>+  nscoord x = nsPresContext::CSSPixelsToAppUnits(aX);
>+  nscoord y = nsPresContext::CSSPixelsToAppUnits(aY);
>+  nsPoint pt(x, y);
>+
>+  nsIPresShell *ps = GetShell();
>+  NS_ENSURE_STATE(ps);
The spec doesn't actually specify this case.
The most logical thing is to return null. I'll file a spec bug.
So
if (!ps) {
  return NS_OK;
}

>+  nsIFrame *rootFrame = ps->GetRootFrame();
>+
>+  // XUL docs, unlike HTML, have no frame tree until everything's done loading
>+  if (!rootFrame)
>+    return NS_OK; // return null to premature XUL callers as a reminder to wait
Nit,
if (expr) {
  stmt;
}

>+
>+  nsIFrame *ptFrame = nsLayoutUtils::GetFrameForPoint(rootFrame, pt, PR_TRUE,
>+                                                      PR_FALSE);
>+  if (!ptFrame)
>+    return NS_OK;
Ditto.

this really needs tests.
r=me if you add tests.
Comment 38 Brad Lassey [:blassey] (use needinfo?) 2011-08-08 08:23:35 PDT
Created attachment 551464 [details] [diff] [review]
patch, ready to land

updated patch for smaug's review
Comment 39 Brad Lassey [:blassey] (use needinfo?) 2011-08-08 08:26:17 PDT
Created attachment 551465 [details] [diff] [review]
test patch, WIP

Here's what I"ve got for a test, however it fails with:
Passed: 0
Failed: 1
Todo: 0
failed | [SimpleTest/SimpleTest.js, window.onerror] - An error occurred: Permission denied for <http://mochi.test:8888> to create wrapper for object of class UnnamedClass at http://mochi.test:8888/tests/content/base/test/bug654352.html:26

in addition, I get 1 warning and 1 error on my console:
Use of enablePrivilege is deprecated.  Please use code that runs with the system principal (e.g. an extension) instead.
http://mochi.test:8888/tests/content/base/test/bug654352.html
0
Permission denied for <http://mochi.test:8888> to create wrapper for object of class UnnamedClass
http://mochi.test:8888/tests/content/base/test/bug654352.html
26
Comment 40 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-08 08:31:53 PDT
Oh, one thing, quite major one. The patch doesn't handle native anonymous content.
So if the position is inside input element, the native anonymous element
shouldn't be returned, but the input element.
Comment 41 Brad Lassey [:blassey] (use needinfo?) 2011-08-08 09:42:48 PDT
(In reply to Olli Pettay [:smaug] from comment #40)
> Oh, one thing, quite major one. The patch doesn't handle native anonymous
> content.
> So if the position is inside input element, the native anonymous element
> shouldn't be returned, but the input element.

I don't know what to do with this
Comment 42 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-08-08 14:23:59 PDT
If the node is anonymous, you need to find the nearest non-anonymous ancestor and return that. You'll also need to return an adjusted offset. You could check NODE_IS_IN_ANONYMOUS_SUBTREE.

Not sure what to do about XBL though.
Comment 43 Brad Lassey [:blassey] (use needinfo?) 2011-08-08 14:31:18 PDT
so, walking up the content nodes to find a non-anonymous one is relatively strait forward. But how do you adjust the offset?
Comment 44 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-08 14:41:17 PDT
XBL isn't really available to web content, so we don't probably need
to handle that in any special way.

For native anonymous, we probably need to special case the cases we care
about: input and textarea. They have internal <div> and the offset is related
to that. So we can probably just keep the offset but change the node to
input/textarea.
note, the spec does also specify few cases when we should return null.
IMO, if the point points to any scrollbar, we should return null. 
Same probably with video controls. Both those are native anonymous.
Comment 45 Jonas Sicking (:sicking) PTO Until July 5th 2011-08-08 14:47:23 PDT
We also need to ensure that the handling for <video> is correct. I think the way it uses XBL results in all video controls being native anonymous, but it's worth checking.
Comment 46 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-08-08 14:49:25 PDT
(In reply to Olli Pettay [:smaug] from comment #44)
> XBL isn't really available to web content, so we don't probably need
> to handle that in any special way.

Marquee uses xbl bindings.
Comment 47 Brad Lassey [:blassey] (use needinfo?) 2011-08-11 23:22:00 PDT
> failed | [SimpleTest/SimpleTest.js, window.onerror] - An error occurred:
> Permission denied for <http://mochi.test:8888> to create wrapper for object
> of class UnnamedClass at
> http://mochi.test:8888/tests/content/base/test/bug654352.html:26
Is this just an error in the dom class info stuff? does this need flags other than DOM_DEFAULT_SCRIPTABLE_FLAGS?
Comment 48 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-12 02:57:12 PDT
DOM_DEFAULT_SCRIPTABLE_FLAGS should be enough, but now I see,
you miss the classinfo stuff in NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsDOMCaretPosition)
Comment 49 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-12 02:58:13 PDT
NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(CaretPosition)
Comment 50 Brad Lassey [:blassey] (use needinfo?) 2011-08-12 06:38:34 PDT
Created attachment 552644 [details] [diff] [review]
patch w/test and anon content handling
Comment 51 Brad Lassey [:blassey] (use needinfo?) 2011-08-15 22:36:49 PDT
Created attachment 553374 [details] [diff] [review]
patch

added anonymous content to the test, which revealed a flaw in this implementation. The spec specifies unsigned long for the offset. In gecko we use signed offsets. 

Seems like we should implement this with signed longs and push for the spec to be updated accordingly.
Comment 52 :Ms2ger 2011-08-16 02:22:17 PDT
(In reply to Brad Lassey [:blassey][blassey@mozilla.com] from comment #51)
> Created attachment 553374 [details] [diff] [review]
> patch
> 
> added anonymous content to the test, which revealed a flaw in this
> implementation. The spec specifies unsigned long for the offset. In gecko we
> use signed offsets. 
> 
> Seems like we should implement this with signed longs and push for the spec
> to be updated accordingly.

Why? Can the offset be negative?
Comment 53 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-16 02:47:33 PDT
Comment on attachment 553374 [details] [diff] [review]
patch


>+  nsFrame::ContentOffsets offsets = ptFrame->GetContentOffsetsFromPoint(pt);
>+  nsCOMPtr<nsIDOMNode> node = do_QueryInterface(offsets.content);
>+  nsIContent* ptContent = offsets.content;
>+  PRInt32 offset = offsets.offset;
>+  if (ptContent &&  ptContent->IsInAnonymousSubtree()) {
>+    nsTArray<nsIContent*> ancestors;
>+    nsTArray<PRInt32> nodeOffsets;
>+    nsContentUtils::GetAncestorsAndOffsets(node, offset, &ancestors, &nodeOffsets);
>+    PRInt32 index;
>+    for (index = 1; index < ancestors.Length() && index < nodeOffsets.Length() && 
>+           ancestors[index] && ancestors[index]->IsInAnonymousSubtree(); index++);
>+    node = do_QueryInterface(ancestors[index]);
>+    offset = nodeOffsets[index];
>+  }


So why is this correct when offsets.content is in native anonymous content of an input element?
Doesn't offset always end up being -1 in that case? It should be the position of the caret in
the input element (if mouse was clicked at the point).
Comment 54 Brad Lassey [:blassey] (use needinfo?) 2011-08-16 07:18:46 PDT
(In reply to Ms2ger from comment #52)
> Why? Can the offset be negative?

The offset returned from GetAncestorsAndOffsets() for the first non-anonymous node is negative when the point is over an <input type="text"/>.

(In reply to Olli Pettay [:smaug] from comment #53)
> So why is this correct when offsets.content is in native anonymous content
> of an input element?
Is it correct? These functions aren't exactly documented well.
> Doesn't offset always end up being -1 in that case? It should be the
> position of the caret in
> the input element (if mouse was clicked at the point).
The offset seems to be -1 consistently for my test. What should it be? and how would you determine it pragmatically?
Comment 55 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-16 07:28:50 PDT
From the spec

"If at the coordinates x,y in the viewport a text insertion point indicator would have been inserted in a text entry widget which is also a replaced element return a caret position with its properties set as follows:

caret node

    The node corresponding to the text entry widget.
caret offset

    The amount of 16-bit units to the left of where the text insertion point indicator would have inserted."
Comment 56 Brad Lassey [:blassey] (use needinfo?) 2011-08-16 07:39:55 PDT
so you're saying that input boxes and text areas need special handling? Why doesn't GetAncestorsAndOffsets() return the right thing?
Comment 57 Brad Lassey [:blassey] (use needinfo?) 2011-08-16 18:15:25 PDT
Trying to special case input fields

  if (offset <= 0 ) {
      offset = 0;
      PRBool isText;
      nsITextControlFrame* textControlFrame = nsnull;
      nsCOMPtr<nsGenericHTMLElement> el;
      printf("%d\n", __LINE__);
      nsCOMPtr<nsIDOMHTMLInputElement> input = do_QueryInterface(node);
      if ( input &&
           NS_SUCCEEDED(input->MozIsTextField(PR_FALSE, &isText)) && isText ) {
        nsIntPoint cursor(x, y);
        nsIWidget* window = rootFrame->GetNearestWidget()->GetTopLevelWidget(); 
        nsQueryContentEvent charAtPt(PR_TRUE, NS_QUERY_CHARACTER_AT_POINT, window);
        charAtPt.time = PR_IntervalNow();
        nsEventStatus status;
        window->DispatchEvent(&charAtPt, status);
        if (charAtPt.mSucceeded)
          offset = charAtPt.mReply.mOffset;
        else
          printf("mSucceeded is false, offset is %d\n", charAtPt.mReply.mOffset);
      }
    }
  }
however its failing here [https://mxr.mozilla.org/mozilla-central/source/content/events/src/nsContentEventHandler.cpp#99] during the dispatch which means we don't get anything meaningful from the query. Any ideas?
Comment 58 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-17 03:53:00 PDT
I don't understand what comment 57 tries to do.

The attached patch is mostly ok, but it handles native anonymous content
in wrong way. If the position points to Text node inside a
text form control, you could use the offset related to text node.

One thing to check: How should the API work when the text is rtl;
Comment 59 Brad Lassey [:blassey] (use needinfo?) 2011-08-17 06:55:53 PDT
Created attachment 553751 [details] [diff] [review]
patch
Comment 60 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-18 06:49:17 PDT
Comment on attachment 553751 [details] [diff] [review]
patch


>+  nsFrame::ContentOffsets offsets = ptFrame->GetContentOffsetsFromPoint(pt);
>+  nsCOMPtr<nsIDOMNode> node = do_QueryInterface(offsets.content);
>+  nsIContent* ptContent = offsets.content;
>+  PRInt32 offset = offsets.offset;
>+  if (ptContent &&  ptContent->IsInAnonymousSubtree()) {
IsInNativeAnonymousSubtree()
And then you could just use
ptContent->FindFirstNonNativeAnonymous()
and if that returns input element (which is MozIsTextField) or textarea,
you could return offset
Otherwise returned node should be null, and offset 0.
Comment 61 Brad Lassey [:blassey] (use needinfo?) 2011-08-18 08:38:21 PDT
Created attachment 554096 [details] [diff] [review]
patch
Comment 62 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-18 11:45:51 PDT
Comment on attachment 554096 [details] [diff] [review]
patch


>+/*  if (ptContent &&  ptContent->IsInAnonymousSubtree()) {
>+    nsTArray<nsIContent*> ancestors;
>+    nsTArray<PRInt32> nodeOffsets;
>+    nsContentUtils::GetAncestorsAndOffsets(node, offset, &ancestors, &nodeOffsets);
>+    PRInt32 index;
>+    for (index = 1; index < ancestors.Length() && index < nodeOffsets.Length() && 
>+           ancestors[index] && ancestors[index]->IsInAnonymousSubtree(); index++);
>+    node = do_QueryInterface(ancestors[index]);
>+    nsCOMPtr<nsIDOMHTMLInputElement> input = do_QueryInterface(node);
>+    nsCOMPtr<nsIDOMHTMLTextAreaElement> textArea = do_QueryInterface(node);
>+    PRBool isText;
>+    if (!textArea &&  !input ||
>+        NS_FAILED(input->MozIsTextField(PR_FALSE, &isText)) || !isText)
>+      offset = nodeOffsets[index];
>+      }*/
I assume you'll remove this.
Comment 63 :Ms2ger 2011-08-18 11:57:42 PDT
Comment on attachment 554096 [details] [diff] [review]
patch

>--- /dev/null
>+++ b/content/base/src/nsDOMCaretPosition.h

>+#ifndef NSDOMCARETPOSITION_H
>+#define NSDOMCARETPOSITION_H

Please use nsDOMCaretPosition_h.

>--- a/content/base/src/nsDocument.cpp
>+++ b/content/base/src/nsDocument.cpp

>+  if (ptContent &&  ptContent->IsInNativeAnonymousSubtree()) {

Just one space will do after the &&

>+    if (textArea ||  input &&
>+        NS_SUCCEEDED(input->MozIsTextField(PR_FALSE, &isText)) && isText) {

And here after the ||. Please put parens around the second clause to clarify the operator precedence.

>--- a/content/base/test/Makefile.in
>+++ b/content/base/test/Makefile.in
>@@ -503,6 +503,7 @@ _TEST_FILES2 = \
> 		test_bug666604.html \
> 		test_bug675121.html \
> 		file_bug675121.sjs \
>+		bug654352.html \

Will this even run without the "test_" in front?

And then land it, before I find more to complain about :)
Comment 64 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-18 12:38:57 PDT
> >+		bug654352.html \
> 
> Will this even run without the "test_" in front?

Good catch! Thanks!
Comment 65 Brad Lassey [:blassey] (use needinfo?) 2011-08-18 13:58:12 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/70b03a8b5a95
Comment 66 christian 2011-08-18 16:21:10 PDT
We had a long discussion about this bug and bug 667243 for mozilla-aurora. In the end we decided we would take it on Aurora ifdef'd for mobile-only and one central we'll take it without them.
Comment 67 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-08-18 18:50:48 PDT
I just realized we should probably implement this with a moz prefix for now. Anne, what do you think?
Comment 69 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-08-19 05:20:29 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #67)
> I just realized we should probably implement this with a moz prefix for now.

Yeah, I really think we should. So does Olli. Brad, can you do it?
Comment 70 Brad Lassey [:blassey] (use needinfo?) 2011-08-19 10:32:35 PDT
Created attachment 554464 [details] [diff] [review]
patch to moz prefix
Comment 71 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-08-19 12:04:40 PDT
Comment on attachment 554464 [details] [diff] [review]
patch to moz prefix


>-  nsIDOMCaretPosition caretPositionFromPoint(in float x, in float y);
>+  nsIDOMCaretPosition mozCaretPositionFromPoint(in float x, in float y);
Update the iid of nsIDOMDocument


Thanks!
Comment 72 :Ms2ger 2011-08-19 12:10:12 PDT
(In reply to Olli Pettay [:smaug] from comment #71)
> Comment on attachment 554464 [details] [diff] [review]
> patch to moz prefix
> 
> 
> >-  nsIDOMCaretPosition caretPositionFromPoint(in float x, in float y);
> >+  nsIDOMCaretPosition mozCaretPositionFromPoint(in float x, in float y);
> Update the iid of nsIDOMDocument

And its subclasses.
Comment 73 Brad Lassey [:blassey] (use needinfo?) 2011-08-19 12:17:13 PDT
(In reply to Ms2ger from comment #72)
> And its subclasses.
Whoa, really? I don't recall ever having to do that before.
Comment 74 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-08-19 13:46:50 PDT
Yeah, we discovered that not revving nsIDOMHTMLDocument when nsIDOMDocument changes causes binary extension crashes.

You can use http://people.mozilla.org/~sfink/uploads/update-uuids to do the work for you.
Comment 75 Trif Andrei-Alin[:AlinT] 2011-08-22 02:35:01 PDT
Hey guys could you give me some steps on how i could verify this issue?
Thanks.
Comment 76 Anne (:annevk) 2011-08-22 10:27:33 PDT
This is already in WebKit. Don't really see the need for a prefix here.
Comment 77 Brad Lassey [:blassey] (use needinfo?) 2011-08-22 12:48:28 PDT
(In reply to Anne van Kesteren from comment #76)
> This is already in WebKit. Don't really see the need for a prefix here.

ok, not landing the prefix patch then
Comment 78 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-08-22 17:06:44 PDT
Actually WebKit has caretRangeFromPoint. But OK, at least we checked :-).
Comment 79 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-08-23 09:46:34 PDT
I wonder what the caret range is doing and if it's accessible: http://www.w3.org/TR/cssom-view/#caret-range
Comment 80 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-23 14:31:27 PDT
Comment on attachment 554096 [details] [diff] [review]
patch

Needs more baking on trunk
Comment 81 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-08-23 18:12:34 PDT
(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #79)
> I wonder what the caret range is doing and if it's accessible:
> http://www.w3.org/TR/cssom-view/#caret-range

What do you mean?
Comment 82 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-08-24 00:38:42 PDT
I doesn't matter, it just doesn't seem to do anything.
Comment 83 Anne (:annevk) 2011-08-30 02:53:04 PDT
I did not realize WebKit still had the older version and never updated. Lame. Without prefix still seems fine though.

Martijn, it is used to define behavior of the method.
Comment 84 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-10-01 02:01:42 PDT
We should back this out from Aurora.
Using Bug 681387 to track that.
Comment 85 :Ehsan Akhgari (out sick) 2011-10-07 13:03:09 PDT
This is going to be backed out, so it should no longer affect add-on compatibility.
Comment 86 Scott Johnson (:jwir3) 2012-12-18 10:09:36 PST
Created attachment 693483 [details] [diff] [review]
b654352 (v2)

I'm pushing this for preliminary review. Aside from un-bitrotting (and administrative stuff like removing PR types and adjusting the boilerplate), I added lines so that offsets are put in frame-relative coordinates. GetOffsetsFromPoint() expects that the coordinates passed into it are frame-relative, which they weren't prior to this new, improved patch. Hence, the problems. 

I've verified that this works as expected with the test cases from bug 681387, BUT I've been unable to get automated tests in for these cases yet. I can't seem to get the mouse click events, when generated via an initEvent() call in JS to call the selection handler code. :( This sucks, because, unless I'm missing something, we can only add automated tests for the kind of test cases Martijn brought up in that bug by changing the selection using an API. This defeats the purpose of verifying that the caretPositionFromPoint and selection focusOffset and focusNode return the same things for a given point. 

Any assistance in this area would be helpful.
Comment 87 Scott Johnson (:jwir3) 2012-12-18 15:14:00 PST
Created attachment 693642 [details] [diff] [review]
b654352 (v3, now with more tests)

Added tests for the new aspects, and modified the old test to test the (now) correct values.
Comment 88 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-12-19 02:40:55 PST
(In reply to Scott Johnson (:jwir3) from comment #86)
> I've verified that this works as expected with the test cases from bug
> 681387, BUT I've been unable to get automated tests in for these cases yet.
> I can't seem to get the mouse click events, when generated via an
> initEvent() call in JS to call the selection handler code. :( This sucks,
> because, unless I'm missing something, we can only add automated tests for
> the kind of test cases Martijn brought up in that bug by changing the
> selection using an API. This defeats the purpose of verifying that the
> caretPositionFromPoint and selection focusOffset and focusNode return the
> same things for a given point. 

Try using EventUtils.js and synthesizeMouse()? That should perform all mouse-related operations including selection.
Comment 89 Jim Mathies [:jimm] 2012-12-19 07:23:53 PST
Comment on attachment 693642 [details] [diff] [review]
b654352 (v3, now with more tests)

Review of attachment 693642 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/interfaces/core/nsIDOMDocument.idl
@@ +367,5 @@
>     * @see <http://dvcs.w3.org/hg/pointerlock/raw-file/default/index.html>
>     */
>    readonly attribute nsIDOMElement mozPointerLockElement;
>  
> +  nsIDOMCaretPosition caretPositionFromPoint(in float x, in float y);

A comment header here explaining what this api does would be helpful.
Comment 90 Scott Johnson (:jwir3) 2012-12-20 13:34:50 PST
Created attachment 694551 [details] [diff] [review]
b654352 (v4)

Converted to webidl. Sending over to Ms2ger for final review on these bindings, then I will push to m-c.

(Along with the moz-prefix patch, too).
Comment 91 :Ms2ger 2012-12-21 00:07:43 PST
Comment on attachment 694551 [details] [diff] [review]
b654352 (v4)

Review of attachment 694551 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but I've found a few small issues still :)

I think you'll need to add to dom/tests/mochitest/general/test_interfaces.html too.

::: content/base/src/nsDOMCaretPosition.cpp
@@ +8,5 @@
> +
> +nsDOMCaretPosition::nsDOMCaretPosition(nsINode* aNode, uint32_t aOffset)
> +  : mOffset(aOffset), mOffsetNode(aNode)
> +{
> +  SetIsDOMBinding();

Assert that mOffsetNode is null, or make it nullable in the IDL file (but then you'll need to call the getter 'GetOffsetNode' instead of just 'OffsetNode').

Yeah, you'll need to make it nullable. Add a note in the IDL that that's necessary because of anonymous content, please.

::: content/base/src/nsDOMCaretPosition.h
@@ +15,5 @@
> + * that node, in the DOM tree.
> + *
> + * http://www.w3.org/TR/cssom-view/#dom-documentview-caretrangefrompoint
> + *
> + * @see nsDOMDocument::caretPositionFromPoint(float x, float y)

nsDOMDocument isn't a thing, afaik. Document:: is fine

@@ +41,5 @@
> +   * node that lies at the point specified.
> +   *
> +   * @returns The DOM node of the CaretPosition.
> +   *
> +   * @see nsDOMDocument::caretPositionFromPoint(float x, float y)

Same here

@@ +47,5 @@
> +  nsINode* OffsetNode();
> +
> +  nsISupports* GetParentObject() const
> +  {
> +    return nullptr;

Return OffsetNode() here.

@@ +50,5 @@
> +  {
> +    return nullptr;
> +  }
> +
> +  virtual JSObject *WrapObject(JSContext *aCx, JSObject *aScope, bool *aTried)

* to the left

::: content/base/src/nsDocument.cpp
@@ +8422,5 @@
> +  if (!rootFrame) {
> +    return NS_OK; // return null to premature XUL callers as a reminder to wait
> +  }
> +
> +  nsIFrame *ptFrame = nsLayoutUtils::GetFrameForPoint(rootFrame, pt, true,

* to the left (three times)

@@ +8437,5 @@
> +  nsFrame::ContentOffsets offsets =
> +    ptFrame->GetContentOffsetsFromPoint(adjustedPoint);
> +
> +  nsCOMPtr<nsIDOMNode> node = do_QueryInterface(offsets.content);
> +  nsIContent* ptContent = offsets.content;

Replace those two lines by

nsCOMPtr<nsIContent> node = offsets.content;

@@ +8447,5 @@
> +    bool isText;
> +    if (textArea || (input &&
> +                     NS_SUCCEEDED(input->MozIsTextField(false, &isText)) &&
> +                     isText)) {
> +      node = do_QueryInterface(nonanon);

and don't QI here

@@ +8455,5 @@
> +    }
> +  }
> +
> +  nsCOMPtr<nsINode> genericNode = do_QueryInterface(node);
> +  *aCaretPos = new nsDOMCaretPosition(genericNode, offset);

And get rid of 'genericNode'; pass 'node' instead.

::: content/base/test/Makefile.in
@@ +503,5 @@
>  		file_html_in_xhr3.html \
>  		file_html_in_xhr.sjs \
>  		test_bug647518.html \
> +		test_bug654352.html \
> +               test_bug654352-2.html \

Looks like you need to use two tabs here :/

::: dom/interfaces/core/nsIDOMDocument.idl
@@ +12,5 @@
>  interface nsIDOMNodeIterator;
>  interface nsIDOMNodeFilter;
>  interface nsIDOMTreeWalker;
>  interface nsIDOMLocation;
> +interface nsIDOMCaretPosition;

Don't need this anymore

@@ +376,5 @@
> +   *          page coordinates.
> +   * @param y Vertical point at which to determine the caret position, in
> +   *          page coordinates.
> +   */
> +  nsISupports caretPositionFromPoint(in float x, in float y);

Make this

nsISupports /* CaretPosition */ caretPositionFromPoint(in float x, in float y);

::: dom/webidl/DOMCaretPosition.webidl
@@ +1,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +interface DOMCaretPosition {

Hm, actually this should be called 'CaretPosition', not 'DOMCaretPosition'. Sorry for not noticing earlier.
Comment 92 Scott Johnson (:jwir3) 2012-12-21 14:37:48 PST
Inbound, with changes requested by Ms2ger and prefixing:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e01ca7212c8a
Comment 93 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-21 16:19:12 PST
Backed out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7c7bc64a114

Your test was failing pretty universally:
32353 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug654352-2.html | expected offset at (115, 35): 11, got: 13
32357 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug654352-2.html | expected offset at (204, 106): 2, got: 8
32358 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug654352-2.html | expected offset at (44, 148): 1, got: 7
32365 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug654352.html | offset in CaretPosition not correct (6== 4)
Comment 94 Scott Johnson (:jwir3) 2012-12-21 16:59:29 PST
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #93)
> Backed out:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/d7c7bc64a114
> 
> Your test was failing pretty universally:
> 32353 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_bug654352-2.html | expected offset at (115,
> 35): 11, got: 13
> 32357 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_bug654352-2.html | expected offset at (204,
> 106): 2, got: 8
> 32358 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_bug654352-2.html | expected offset at (44,
> 148): 1, got: 7
> 32365 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_bug654352.html | offset in CaretPosition not
> correct (6== 4)


Guh... these tests are in need of work. I should've pushed to try. thanks for backing out, Gavin.
Comment 95 :Ms2ger 2012-12-21 23:41:29 PST
Wait, why prefix it?
Comment 96 Scott Johnson (:jwir3) 2012-12-22 10:21:28 PST
(In reply to :Ms2ger from comment #95)
> Wait, why prefix it?

From comment 67, roc and Olli seem to think this should be prefixed.
Comment 97 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-22 12:31:41 PST
That comment significantly predates our new no-prefixing policy (https://groups.google.com/forum/#!topic/mozilla.dev.platform/34JfwyEh5e4).
Comment 98 Scott Johnson (:jwir3) 2012-12-28 09:11:33 PST
Reworked the tests to be a bit more stable, unprefixed (as per comment 97), and landed on inbound again:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3cc198a6b83
Comment 99 Graeme McCutcheon [:graememcc] 2012-12-29 04:29:57 PST
https://hg.mozilla.org/mozilla-central/rev/c3cc198a6b83

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