Closed Bug 503943 Opened 15 years ago Closed 15 years ago

Need a way to grab mouse events on arbitrary elements (implement setCapture/releaseCapture)

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: pavlov, Assigned: enndeakin)

References

(Depends on 3 open bugs)

Details

(Keywords: dev-doc-complete)

Attachments

(4 files, 5 obsolete files)

We need the behavior that scrollbars have, where when you click down on them and leave the window they continue to receive mouse move events and mouse up events.  We need this ability on a <div> element (or any element, really), and would be happy to set it via style or attribute or whatever would be best.
tracking-fennec: --- → 1.0b3+
Currently, it can be done with viewManager->GrabMouseEvents() although it's view-granular not frame-granular. But it would be great if this could be available for a node, possibly just a grabMouse type api that can be called within a mousedown event (and chrome code only) and would get released automatically upon mouseup.
Assignee: nobody → enndeakin
Attached file work in progress (obsolete) —
This patch removes all the view mouse capturing and replaces it with a content node.

I don't like the api for this though. Currently it is:

mouseDownEvent->mozGrabMouse(node)

and the grab is released on mouseup.

Other possibilities include reusing the old captureMouse api or the ie api. Or, since this would be privileged code only anyway, a method on nsIDOMChromeWindow. Whatever the mechanism, it can only be triggered while a real mousedown is being fired.
There's no particular reason why this should be chrome-only, is there? As long as we always remove the capture when the mouse button is released?

Do specs or other browsers have content-accessible mouse capture?
IE uses(In reply to comment #3)
> There's no particular reason why this should be chrome-only, is there? As long
> as we always remove the capture when the mouse button is released?

Don't know. I assumed there was some unsafe reason why we never implemented and/or removed the mouse capturing. But if not, I can make it accessible to content as well.

> Do specs or other browsers have content-accessible mouse capture?

IE uses element.setCapture() and element.releaseCapture()
Attached patch updated patch (obsolete) — Splinter Review
This patch fixes the various issues with the previous patch. Mouse capture should now work for:
 - splitters
 - resizers
 - titlebars
 - frameset borders
 - select dropdowns
 - selection scrolling
 - sliders/scrollbars

The only thing unclear of now is what api to use for this.
Attached patch testSplinter Review
Tests for the above
Attached patch use setCapture/releaseCapture (obsolete) — Splinter Review
This implements the IE API:

element.setCapture(bool) is used to set capture on the element. From that point, all mouse events are targeted at the element. The argument specifies whether events can be targeted at descendants (if false) or not (if true). It may only be called during a mousedown event. 

document.releaseCapture() can be used to remove capture. It will also be removed when the mouse is released.

There are some differences between IE:
- the default value if the argument is omitted defaults to true on IE, but optional arguments for us default to false.
- IE allows calling setCapture at any time. This patch limits it to mousedown only.
- IE also has an releaseCapture() defined for elements as well, but I don't think that it particularly useful.
Attachment #394054 - Flags: review?(roc)
Sounds OK to me. Can we get WHATWG feedback on this?

Would it make sense to support element.releaseCapture() and simply ignore it if the element doesn't have the capture? That would mean you could write script code for an element's event handler that releases capture on some condition and you wouldn't have to explicitly check that it was *your* capture that you're releasing (so you don't accidentally release someone else's capture). Seems like that might be useful to avoid author mistakes.

How does this interact with drag/drop?
Felipe, perhaps this could be extended to support touch and gesture events.
Then the script could say whether it wants all the events to be dispatched to the
same element.
(In reply to comment #8)
> Sounds OK to me. Can we get WHATWG feedback on this?

This was asked for at http://lists.w3.org/Archives/Public/public-webapps/2008OctDec/0308.html

> Would it make sense to support element.releaseCapture() and simply ignore it if
> the element doesn't have the capture? That would mean you could write script
> code for an element's event handler that releases capture on some condition and
> you wouldn't have to explicitly check that it was *your* capture that you're
> releasing (so you don't accidentally release someone else's capture). Seems
> like that might be useful to avoid author mistakes.

I can add support for element.releaseCapture() as well very easily.

> How does this interact with drag/drop?

If a drag is started, the capture is cancelled (in nsBaseDragService::InvokeDragSession) and the drag proceeds as normal.
(In reply to comment #10)
> (In reply to comment #8)
> > Sounds OK to me. Can we get WHATWG feedback on this?
> 
> This was asked for at
> http://lists.w3.org/Archives/Public/public-webapps/2008OctDec/0308.html

Sigh.

> > How does this interact with drag/drop?
> 
> If a drag is started, the capture is cancelled (in
> nsBaseDragService::InvokeDragSession) and the drag proceeds as normal.

OK. I won't be able to review this next week since I'll be away. Maybe during the work week.
(In reply to comment #9)
> Felipe, perhaps this could be extended to support touch and gesture events.
> Then the script could say whether it wants all the events to be dispatched to
> the
> same element.

Olli, this does seem as a great fit for our capturing the events for the touch and gesture events as well. I think the best would be to file a follow up bug and extend this for the touch events after this gets reviewed.

Neil, would it be hard to extend this implementation to work with other events too? In this case it would be MozTouchDown/MozTouchMove and possibly others. I haven't taken a thorough look at the patch yet but it seems it wont be hard to extend this for other events.
You'd just have to call nsIPresShell::ActivateCapturingContent() during nsEventStateManager::PostHandleEvent for the MozTouchDown event. It shouldn't be possible to start capture during a move event.
(In reply to comment #13)
> You'd just have to call nsIPresShell::ActivateCapturingContent() during
> nsEventStateManager::PostHandleEvent for the MozTouchDown event. It shouldn't
> be possible to start capture during a move event.

Ah okay, that's nice. I mentioned MozTouchMove because I thought I would need to specify what events are grabbed, but now I see that every event will be grabbed during setCapture mode and sent to the chosen target.
A question about the patch.
What happens if a script does:
<div onmousedown="this.setCapture(); this.parentNode.removeChild(this);">

What does IE do in that case? Will all the mouse move events still be
dispatched to the <div>?

And another thing. Is it guaranteed that gCaptureInfo.content is released on
shutdown? (Especially once elements start to own their owner document.)
How does the patch handle case like
<div onmousedown="this.style.display = 'none'; this.setCapture();">?
What does IE do?
(In reply to comment #15)
> A question about the patch.
> What happens if a script does:
> <div onmousedown="this.setCapture(); this.parentNode.removeChild(this);">
<div onmousedown="this.style.display = 'none'; this.setCapture();">?

After fixing a crash, in both cases the frame doesn't exist and the capture just gets redirected to the document. I also tested this with removing the content and frame during a timeout.
  
> And another thing. Is it guaranteed that gCaptureInfo.content is released on
> shutdown? (Especially once elements start to own their owner document.)

It should be cleared when the view is destroyed.
Attached patch updated patch (obsolete) — Splinter Review
This patch:
- adds element.releaseCapture()
- fixes a null-check crash in GetActiveSelectionFrame when an element loses its frame
- changes GetCapturingContent to add an extra argument to ignore whether the capture is active. This is needed so that releaseCapture can be called during a mousedown event where the capture will not yet be active. 
- add tests for the above
Attachment #394054 - Attachment is obsolete: true
Attachment #397018 - Flags: review?(roc)
Attachment #394054 - Flags: review?(roc)
+  if (node && nsContentUtils::CanCallerAccess(node))
+    nsIPresShell::SetCapturingContent(nsnull, PR_FALSE);

{}

+   * aIgnoreActive is true, then ignorer whether the capture is active or not.

"then ignore"

+  if (nsIPresShell::GetCapturingContent(PR_TRUE) == mContent)
+    nsIPresShell::SetCapturingContent(nsnull, PR_FALSE);

{}

+  PRPackedBool active; // if true, capturing on gCapturingContent is active
+  PRPackedBool retargetToContainer;
+  nsIContent* content;

mActive etc

+    if (gCaptureInfo.content)
+      gCaptureInfo.active = PR_TRUE;

{}

+  static void SetCapturingContent(nsIContent* aContent,
+                                  PRBool aActive,
+                                  PRBool aRetargetToContainer = PR_FALSE);

default parameters aren't great, nor are boolean parameters, I think it would be better (more readable) to have a single flags word here.

+        // A mousedown event occurred, so make sure that the capturing content is cleared.

This comment should explain in more detail what happened

+   * If aRetargetToContainer true, all mouse events are targeted aContent only.

"are targeted at"

Although, what are the "applicable descendants"? This could be explained more clearly.

+  if (aContent)
+    NS_ADDREF(gCaptureInfo.content = aContent);

{}

It might be better to have GetCapturingContent broken into two methods, GetCapturingContent and GetCapturingContentIgnoringActivation?

+    nsCOMPtr<nsIContent> capturingContent = do_QueryInterface(gCaptureInfo.content);

do_QueryInterface not needed. We don't need an nsCOMPtr here either

+    nsCOMPtr<nsIContent> capturingContent =
+      do_QueryInterface(nsIPresShell::GetCapturingContent(PR_FALSE));

Ditto

+      if (!capturingContent->GetParent())
+        frame = frame->GetParent();
+      else {

{}

+          if (childframe)
+            frame = childframe;

{}

+        if (frame->GetType() == nsGkAtoms::scrollFrame) {
+          nsIScrollableFrame* scrollFrame = do_QueryFrame(frame);

Might as well not do the GetType() check

+          if (scrollFrame)
+            frame = scrollFrame->GetScrolledFrame();

{}

The retargetToContainer argument is
+   * not currently supported.

Really? It looks supported to me. However, it's confusingly named. It causes retargeting from descendants of the element to the element itself, not a container of the element!

+    }
+    else if (NS_IS_MOUSE_EVENT(aEvent) && GetCapturingContent(PR_FALSE)) {

} else

+  // If the mouse is dragged outside the nearest enclosing scrollable area
+  // while making a selection, the area will be scrolled. To do this, capture
+  // the mouse on the nearest scrollable frame. If there isn't a scrollable
+  // frame, or something else is already capturing the mouse, there's no
+  // reason to capture.

We should really change this so that you can drag out of overflowing elements that are already scrolled to the top/bottom, but that's a separate issue.

+    nsCOMPtr<nsIContent> capturingContent =
+      do_QueryInterface(nsIPresShell::GetCapturingContent(PR_FALSE));
+    if (capturingContent)
+      mContent = capturingContent;

Shouldn't this just be mContent = nsIPresShell::GetCapturingContent()?

+  if (viewObserver)
+    viewObserver->ClearMouseCapture(this);

{}

 void nsView::InvalidateHierarchy(nsViewManager *aViewManagerParent)
 {
   if (aViewManagerParent) {
     // We're removed from the view hierarchy of aRemovalPoint, so make sure
     // we're not still grabbing mouse events.
-    if (aViewManagerParent->GetMouseEventGrabber() == this) {
-      PRBool res;
-      aViewManagerParent->GrabMouseEvents(nsnull, res);
-    }
+    DropMouseGrabbing();

Do we still need DropMouseGrabbing here? It seems to me we shouldn't.

+          if (viewObserver)
+            viewObserver->ClearMouseCapture(nsnull);

{}

+    if (viewObserver)
+      viewObserver->ClearMouseCapture(nsnull);

{}

It looks like Web content can override capturing being performed by resizer elements, title elements, framesets, etc, as long as the caller can access the content node. Is that really what we want? I'm a little bit afraid that our frame code is not set up to deal with the capture being stolen at arbitrary times without a mouse-up event.

Other than that, this looks fantastic. Less use of views, lots of code removal, yum!
> +  static void SetCapturingContent(nsIContent* aContent,
> +                                  PRBool aActive,
> +                                  PRBool aRetargetToContainer = PR_FALSE);
> 
> default parameters aren't great, nor are boolean parameters, I think it would
> be better (more readable) to have a single flags word here.
> 

I think it would be simpler to just remove the default value and make it required.

> It looks like Web content can override capturing being performed by resizer
> elements, title elements, framesets, etc, as long as the caller can access the
> content node. Is that really what we want? I'm a little bit afraid that our
> frame code is not set up to deal with the capture being stolen at arbitrary
> times without a mouse-up event.

What about if I just prevent setCapture from doing anything if an active capture is already set?
> > +  static void SetCapturingContent(nsIContent* aContent,
> > +                                  PRBool aActive,
> > +                                  PRBool aRetargetToContainer = PR_FALSE);
> > 
> > default parameters aren't great, nor are boolean parameters, I think it would
> > be better (more readable) to have a single flags word here.
> 
> I think it would be simpler to just remove the default value and make it
> required.

It might be a simpler change, but I think the code really will read much better with named flags.

> > It looks like Web content can override capturing being performed by resizer
> > elements, title elements, framesets, etc, as long as the caller can access the
> > content node. Is that really what we want? I'm a little bit afraid that our
> > frame code is not set up to deal with the capture being stolen at arbitrary
> > times without a mouse-up event.
> 
> What about if I just prevent setCapture from doing anything if an active
> capture is already set?

That would solve this problem, but it's a pretty significant API change. Do you think authors would like to be able to switch capture from one element to another? (I suppose they can do that by releasing capture and then setting it again.) What does IE do in this situation?
Hmm, it's a bit of a problem that capture is global. It would suck if some page calls SetCapture and that means another page can't call SetCapture, or worse, that the first page gets the capture the next time the user clicks anywhere. ("Page" includes IFRAMEs here.) How do you think this should work?
> That would solve this problem, but it's a pretty significant API change. Do you
> think authors would like to be able to switch capture from one element to
> another? (I suppose they can do that by releasing capture and then setting it
> again.) What does IE do in this situation?

I meant that it would only be prevented when an capture has already been activated. The frames use SetCapturingContent(content, PR_TRUE) while a call to element.setCapture passes false such that the capture doesn't become activated until the mousedown event has finished processing. Thus, only the 'built-in' captures would not be overridable.
OK, but someone can still call setCapture at any time and then it will be activated at the next mousedown, right?

And do authors want to be able to setCapture while there is a capture active?
(In reply to comment #24)
> OK, but someone can still call setCapture at any time and then it will be
> activated at the next mousedown, right?

Not sure what you mean here. Can you give an example?
Suppose I load http://example.com/foobar. In onload it does
  document.body.setCapture();
Then I click in somewhere in the Firefox UI. Isn't the capture activated?
OK I see what you mean. setCapture is only supposed to work during a mousedown event. I can add a call in nsEventStateManager::PreHandleEvent to ensure the capture is cleared first.
What if one page does an alert() in onmousedown and a page in another window calls setCapture during that nested event loop?
(In reply to comment #22)
> Hmm, it's a bit of a problem that capture is global.

OK, so what would you suggest here?
Attached patch update for comments (obsolete) — Splinter Review
This patch addresses all of the comments, plus adds a flag when mousedown occurs. When a window is lowered, the view is removed, or a drag begins, the capturing will be disabled until the next mousedown.
Attachment #391762 - Attachment is obsolete: true
Attachment #393024 - Attachment is obsolete: true
Attachment #397018 - Attachment is obsolete: true
Attachment #398205 - Flags: review?(roc)
Attachment #397018 - Flags: review?(roc)
+  gCaptureInfo.mActive = aFlags & CAPTURE_ACTIVE ? PR_TRUE : PR_FALSE;
+  gCaptureInfo.mRetargetToElement = aFlags & CAPTURE_RETARGETTOELEMENT ? PR_TRUE : PR_FALSE;

I think (aFlags & CAPTURE_ACTIVE) != 0 is a better way to write this.

We discussed using the popup blocker's approach to enabling capturing only during a mouse event. Why didn't that work out? This latest patch still has the problem in comment #28 right?
tracking-fennec: 1.0b3+ → ---
The popup blocker uses an object at the beginning of events that is reset when it goes out of scope. There were issues with that technique working with my original imlplementation, but I've since found more problems with alerts and capturing that I decided to rework the code a bit. I moved the enable/disable capture into presshell and changed to start capturing immediately instead of waiting for the end of the event.
Attachment #398205 - Attachment is obsolete: true
Attachment #398691 - Flags: review?(roc)
Attachment #398205 - Flags: review?(roc)
I was thinking you could actually use the popup blocker's own state here.
Attached patch how about this?Splinter Review
Attachment #399204 - Flags: review?(roc)
Comment on attachment 399204 [details] [diff] [review]
how about this?

Yeah!
Attachment #399204 - Flags: review?(roc) → review+
Attachment #399204 - Flags: superreview?(Olli.Pettay)
Hah, it turns out I need this to get rid of some view stuff from nsSelection.cpp!
Comment on attachment 399204 [details] [diff] [review]
how about this?

Yes, this is great!
Attachment #399204 - Flags: superreview?(Olli.Pettay) → superreview+
I checked this in but had to back it out again due to some test failures:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1252594107.1252596545.30687.gz

Unfortunately, I cannot reproduce them locally.
Do they reproduce consistently on the try-server?

In test_bug503813.html it seems the synthetic mouse moves aren't triggering hover... that seems weird, there's no mousedown so we shouldn't be triggering capture code at all...
(In reply to comment #39)
> In test_bug503813.html it seems the synthetic mouse moves aren't triggering
> hover... that seems weird, there's no mousedown so we shouldn't be triggering
> capture code at all...

I figured out the problem with that test. The previous test was firing a mousedown and mousemove but no mouseup.

The scale test fails on the try server build I just posted as well with the same problem, but didn't before that I recall. Can't get it to fail locally though. It's just the parts of the test that this bug adds that fail.
It sounds like you could add logging code and tryserver your way to victory...
Attached patch fix issuesSplinter Review
This patch fixes the issues with the tests and works on the try server on all platforms.

The issue with the scale test on Linux was caused due to some extra gap between the thumb and scale edge that caused the mouse event to fire at the wrong target. On Windows, the thumb snaps back if the mouse is moved too far perpendicular to the scale. I fixed this and added another test specifically to test the snapping behaviour.

I'll just check this in tomorrow.
Checked in and no failures now!

http://hg.mozilla.org/mozilla-central/rev/eda2433181c9
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Could this have caused bug 516722 ?
Depends on: 516880
Depends on: 516991
Depends on: 517737
Depends on: 517916
Depends on: 520013
Depends on: 520655
No longer depends on: 520655
Depends on: 520501
Keywords: dev-doc-needed
Summary: Need a way to grab mouse events on arbitrary elements → Need a way to grab mouse events on arbitrary elements (implement setCapture/releaseCapture)
Target Milestone: --- → mozilla1.9.3a1
Version: unspecified → Trunk
Whiteboard: [doc-waiting-1.9.3]
Depends on: 526403
Depends on: 526769
Depends on: 536486
Depends on: 540491
Depends on: 541011
Depends on: 578667
Depends on: 582771
I think the change discussed here has caused some issues with jQuery UI, scriptaculous and possibly other libraries.

jQuery UI offers a method called draggable that makes any element draggable. In olders versions of firefox as well as in other browsers (Chrome, Opera), when the mouse moves outsides the window while an object is being dragged, then the object continues to move. This doesn't happen under firefox 4.0 beta 6.

The same issue also appears with the draggable method in scriptaculous. You can verify that with the following demo: http://wiki.github.com/madrobby/scriptaculous/draggable

When I insert a setCapture/releaseCapture in the mousedown/mouseup events then everything works fine.

Is this the expected behavior? This breaks compatibility with previous versions of firefox...
Here's a link to a jquery ui demo:
http://jqueryui.com/demos/draggable/
Marios,could you please file a new bug.
And a regression range would be great.
Note, the patch in this bug landed long ago.
(In reply to comment #48)
> Marios,could you please file a new bug.

Bug 603550
These are now documented:

https://developer.mozilla.org/en/DOM/document.setCapture
https://developer.mozilla.org/en/DOM/document.releaseCapture

With a working example linked from those pages. Also mentioned on Fx4 for developers.
Whiteboard: [doc-waiting-1.9.3]
Depends on: 624127
With the absence of this functionality in previous versions of Firefox, we added code that provides our own setCapture and releaseCapture functionality:

if (firefox)
  {
  HTMLElement.prototype.setCapture = function()
  {
    // custom code to support capture 
  };
}

and for example in our code where we have:
document.body.setCapture();

our code gets called, in firefox 3.6.


now with Firefox 4 and the setCapture functionality added, our function is no longer called and we have problems. 
I would have thought that our declaration of HTMLElement.prototype.setCapture above would overide the support now provided by Firefox 4 but that is not the case.
How can I override this new API in Firefox 4? 
Ultimately I might want to call the native code like so:

HTMLElement.prototype._setCapture = HTMLElement.prototype.setCapture; 

  HTMLElement.prototype.setCapture = function()
  {
    // custom code to support capture 

    // now call native code
    this._setCapture();
  };

..but right now I am trying to get the custom handler to be called.
Any suggestions?
Jeff, you problem is that right now we put all DOM-defined things on the most-derived DOME prototype.  We're planning on changing that, but it hasn't happened yet.

So there is a setCapture on HTMLBodyElement.prototype, in particular.

As long as you know you're working with a <body>, overriding it on HTMLBodyElement will do.  If not, you have to do it for all element classes...
Thanks for the super-quick response Boris.
You are correct - HTMLBodyElement.prototype works for this particular case. However, we have many calls to setCapture/releaseCapture and they are not all related to a body. So as you mention, we would have to provided for all element classes.
You mention that there are plans to change where these DOM prototypes are defined. Would that be in an upcoming RC candidate? If not, do you know when?

,Jeff
Jeff, those plans are not all that short-term; they require entirely redoing how our DOM is reflected into JavaScript.  Probably Firefox 6 (in about 6 months) if everything goes well....
Depends on: 647922
Depends on: 973604
Depends on: 623432
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.