703150 - Cannot drag scrollbar thumb if mousedown event is called stopPropagation()

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Load the URL and try dragging the scrollbar thumb, but you cannot do it.

I'll post a patch soon.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

I guess there are similar bugs, but they should be fixed separated bugs...
http://mxr.mozilla.org/mozilla-central/search?string=AddEventListener%28&case=1&find=\.cpp%24&findi=\.c&filter=^*%2F%28layout|content%29%2F[^\0]*%24&hitlimit=&tree=mozilla-central

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Attachment: Patch

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 575106 [details] [diff] [review]
Patch


>+   * AddSystemEventListener() adds an event listener of aType to the system
>+   * group.  Typically, core code should use system group for listening to any
>+   * events.  If core code uses nsIDOMEventTarget::AddEventListener, it means
>+   * that the listener cannot listen the event when web content calls
>+   * stopPropagation() of the event.
>+   *
>+   * @param aType            An event name you're going to handle.
>+   * @param aListener        An event listener.
>+   * @param aUseCapture      TRUE if you want to listen the event in capturing
>+   *                         phase.  Otherwise, FALSE.
>+   * @param aWantsUntrusted  TRUE if you want to handle untrusted events.
>+   *                         Otherwise, FALSE.
>+   * @return                 NS_OK if succeed.  Otherwise, NS_ERROR_*.
>+   */
>+  nsresult AddSystemEventListener(const nsAString& aType,
>+                                  nsIDOMEventListener *aListener,
>+                                  bool aUseCapture,
>+                                  bool aWantsUntrusted = false);
>+
>+  /**
>+   * RemoveSystemEventListener() should be used if you have used
>+   * AddSystemEventListener().
>+   */
>+  nsresult RemoveSystemEventListener(const nsAString& aType,
>+                                     nsIDOMEventListener *aListener,
>+                                     bool aUseCapture);

Would be nice to have these in nsIDOMEventTarget (in the C++ section).
And should the methods be virtual so that binary addons could use them.

I think having then nsIDOMEvenTarget::Add/RemoveSystemEventListener(...) {} in nsGenericElement.cpp 
would be reasonable option.

Would that work?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Attachment: Patch

Implementing AddSystemEventListener() like AddEventListener().

If we used C++ section, we need to change every header file whose class inherits nsIDOMEventTarget.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Attachment: Patch

fix a nit.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Oh, I realized that preventDefault() of mousedown event doesn't prevent the scrollbar operation. On WebKit, web applications can do preventDefault() for scrollbar. But I'm not sure whether that is intentional behavior or not.

Anyway, it's another bug...

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

FYI: The WebKit's behavior is unintentional. I can prevent the scrollbar behavior only when call preventDefault() at a handler which listens window, document or document.documentElement. And I cannot prevent the scrollbar operation of overflow:auto;'s.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 575389 [details] [diff] [review]
Patch


>+nsWindowRoot::RemoveSystemEventListener(const nsAString& aType,
>+                                        nsIDOMEventListener* aListener,
>+                                        bool aUseCapture)
>+{
>+  nsEventListenerManager* listenerManager = GetListenerManager(false);
>+  if (!listenerManager) {
>+    return NS_OK;
>+  }
>+  PRUint32 flags = NS_EVENT_FLAG_SYSTEM_EVENT;
>+  flags |= aUseCapture ? NS_EVENT_FLAG_CAPTURE : NS_EVENT_FLAG_BUBBLE;
>+  listenerManager->RemoveEventListenerByType(aListener, aType, flags);
>+  return NS_OK;
>+}
There is quite a bit copy-pasting.
Could you perhaps add some static helper methods to nsEventListenerManager.
AddSystemEventListener needs probably some different implementations, but
for RemoveSystemEventListener there could be even some macro to forward the
method call to ELM.



>+  /**
>+   * addSystemEventListener() adds an event listener of aType to the system
>+   * group.  Typically, core code should use system group for listening to any
>+   * events.  If core code uses nsIDOMEventTarget::AddEventListener, it means
>+   * that the listener cannot listen the event when web content calls
>+   * stopPropagation() of the event.
Not quite true. chrome can get access to the event if it uses capturing event listener, 
and adds the listener to chromeEventListener or some other chrome event target.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Attachment: Patch

I have no idea a good name for the helper method for implementing AddSystemEventListener() if the method is a static method in nsEventListerManager. Therefore, I made it as a inline method in nsEventListenerManager.h.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 575711 [details] [diff] [review]
Patch

># HG changeset patch
># Parent 1695164143492a4890c4d63af48b70f3c25f2d1e
>Bug 703150 Cannot drag scrollbar thumb if stopPropagation() of mousedown event is called
>
>diff --git a/content/base/public/nsINode.h b/content/base/public/nsINode.h
>--- a/content/base/public/nsINode.h
>+++ b/content/base/public/nsINode.h
>@@ -283,18 +283,18 @@ private:
> // Categories of node properties
> // 0 is global.
> #define DOM_USER_DATA         1
> #define DOM_USER_DATA_HANDLER 2
> #define SMIL_MAPPED_ATTR_ANIMVAL 3
> 
> // IID for the nsINode interface
> #define NS_INODE_IID \
>-{ 0x20d16be2, 0x3c58, 0x4099, \
>-  { 0xbf, 0xa6, 0xd0, 0xe7, 0x6b, 0xb1, 0x3d, 0xc5 } }
>+{ 0xd026d280, 0x5b25, 0x41c0, \
>+  { 0x92, 0xcf, 0x6, 0xf6, 0xf, 0xb, 0x9a, 0xfe } }
> 
> /**
>  * An internal interface that abstracts some DOMNode-related parts that both
>  * nsIContent and nsIDocument share.  An instance of this interface has a list
>  * of nsIContent children and provides access to them.
>  */
> class nsINode : public nsIDOMEventTarget,
>                 public nsWrapperCache
>@@ -723,16 +723,17 @@ public:
>     return mParent && mParent->IsElement() ? mParent : nsnull;
>   }
> 
>   /**
>    * See nsIDOMEventTarget
>    */
>   NS_DECL_NSIDOMEVENTTARGET
>   using nsIDOMEventTarget::AddEventListener;
>+  using nsIDOMEventTarget::AddSystemEventListener;
> 
>   /**
>    * Adds a mutation observer to be notified when this node, or any of its
>    * descendants, are modified. The node will hold a weak reference to the
>    * observer, which means that it is the responsibility of the observer to
>    * remove itself in case it dies before the node.  If an observer is added
>    * while observers are being notified, it may also be notified.  In general,
>    * adding observers while inside a notification is not a good idea.  An
>diff --git a/content/base/src/nsGenericElement.cpp b/content/base/src/nsGenericElement.cpp
>--- a/content/base/src/nsGenericElement.cpp
>+++ b/content/base/src/nsGenericElement.cpp
>@@ -1092,27 +1092,51 @@ nsINode::AddEventListener(const nsAStrin
>   nsEventListenerManager* listener_manager = GetListenerManager(true);
>   NS_ENSURE_STATE(listener_manager);
>   listener_manager->AddEventListener(aType, aListener, aUseCapture,
>                                      aWantsUntrusted);
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>+nsINode::AddSystemEventListener(const nsAString& aType,
>+                                nsIDOMEventListener *aListener,
>+                                bool aUseCapture,
>+                                bool aWantsUntrusted,
>+                                PRUint8 aOptionalArgc)
>+{
>+  NS_ASSERTION(!aWantsUntrusted || aOptionalArgc > 1,
>+               "Won't check if this is chrome, you want to set "
>+               "aWantsUntrusted to false or make the aWantsUntrusted "
>+               "explicit by making aOptionalArgc non-zero.");
>+
>+  if (!aWantsUntrusted &&
>+      (aOptionalArgc < 2 &&
>+       !nsContentUtils::IsChromeDoc(OwnerDoc()))) {
>+    aWantsUntrusted = true;
>+  }
>+
>+  return NS_AddSystemEventListener(this, aType, aListener, aUseCapture,
>+                                   aWantsUntrusted);
>+}
>+
>+NS_IMETHODIMP
> nsINode::RemoveEventListener(const nsAString& aType,
>                              nsIDOMEventListener* aListener,
>                              bool aUseCapture)
> {
>   nsEventListenerManager* elm = GetListenerManager(false);
>   if (elm) {
>     elm->RemoveEventListener(aType, aListener, aUseCapture);
>   }
>   return NS_OK;
> }
> 
>+NS_IMPL_REMOVE_SYSTEM_EVENT_LISTENER(nsINode)
>+
> nsresult
> nsINode::PreHandleEvent(nsEventChainPreVisitor& aVisitor)
> {
>   // This is only here so that we can use the NS_DECL_NSIDOMTARGET macro
>   NS_ABORT();
>   return NS_ERROR_NOT_IMPLEMENTED;
> }
> 
>diff --git a/content/events/src/nsDOMEventTargetHelper.cpp b/content/events/src/nsDOMEventTargetHelper.cpp
>--- a/content/events/src/nsDOMEventTargetHelper.cpp
>+++ b/content/events/src/nsDOMEventTargetHelper.cpp
>@@ -98,16 +98,18 @@ nsDOMEventTargetHelper::RemoveEventListe
>   nsEventListenerManager* elm = GetListenerManager(false);
>   if (elm) {
>     elm->RemoveEventListener(aType, aListener, aUseCapture);
>   }
> 
>   return NS_OK;
> }
> 
>+NS_IMPL_REMOVE_SYSTEM_EVENT_LISTENER(nsDOMEventTargetHelper)
>+
> NS_IMETHODIMP
> nsDOMEventTargetHelper::AddEventListener(const nsAString& aType,
>                                          nsIDOMEventListener *aListener,
>                                          bool aUseCapture,
>                                          bool aWantsUntrusted,
>                                          PRUint8 aOptionalArgc)
> {
>   NS_ASSERTION(!aWantsUntrusted || aOptionalArgc > 1,
>@@ -126,16 +128,41 @@ nsDOMEventTargetHelper::AddEventListener
> 
>   nsEventListenerManager* elm = GetListenerManager(true);
>   NS_ENSURE_STATE(elm);
>   elm->AddEventListener(aType, aListener, aUseCapture, aWantsUntrusted);
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>+nsDOMEventTargetHelper::AddSystemEventListener(const nsAString& aType,
>+                                               nsIDOMEventListener *aListener,
>+                                               bool aUseCapture,
>+                                               bool aWantsUntrusted,
>+                                               PRUint8 aOptionalArgc)
>+{
>+  NS_ASSERTION(!aWantsUntrusted || aOptionalArgc > 1,
>+               "Won't check if this is chrome, you want to set "
>+               "aWantsUntrusted to false or make the aWantsUntrusted "
>+               "explicit by making aOptionalArgc non-zero.");
>+
>+  if (aOptionalArgc < 2) {
>+    nsresult rv;
>+    nsIScriptContext* context = GetContextForEventHandlers(&rv);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    nsCOMPtr<nsIDocument> doc =
>+      nsContentUtils::GetDocumentFromScriptContext(context);
>+    aWantsUntrusted = doc && !nsContentUtils::IsChromeDoc(doc);
>+  }
>+
>+  return NS_AddSystemEventListener(this, aType, aListener, aUseCapture,
>+                                   aWantsUntrusted);
>+}
>+
>+NS_IMETHODIMP
> nsDOMEventTargetHelper::DispatchEvent(nsIDOMEvent* aEvent, bool* aRetVal)
> {
>   nsEventStatus status = nsEventStatus_eIgnore;
>   nsresult rv =
>     nsEventDispatcher::DispatchDOMEvent(this, nsnull, aEvent, nsnull, &status);
> 
>   *aRetVal = (status != nsEventStatus_eConsumeNoDefault);
>   return rv;
>diff --git a/content/events/src/nsEventListenerManager.h b/content/events/src/nsEventListenerManager.h
>--- a/content/events/src/nsEventListenerManager.h
>+++ b/content/events/src/nsEventListenerManager.h
>@@ -313,9 +313,31 @@ protected:
> 
>   static PRUint32                           mInstanceCount;
>   static jsid                               sAddListenerID;
> 
>   friend class nsEventTargetChainItem;
>   static PRUint32                           sCreatedCount;
> };
> 
>+/**
>+ * NS_AddSystemEventListener() is a helper function for implementing
>+ * nsIDOMEventTarget::AddSystemEventListener().
>+ */
>+inline nsresult
>+NS_AddSystemEventListener(nsIDOMEventTarget* aTarget,
>+                          const nsAString& aType,
>+                          nsIDOMEventListener *aListener,
>+                          bool aUseCapture,
>+                          bool aWantsUntrusted)
>+{
>+  nsEventListenerManager* listenerManager = aTarget->GetListenerManager(true);
>+  NS_ENSURE_STATE(listenerManager);
>+  PRUint32 flags = NS_EVENT_FLAG_SYSTEM_EVENT;
>+  flags |= aUseCapture ? NS_EVENT_FLAG_CAPTURE : NS_EVENT_FLAG_BUBBLE;
>+  if (aWantsUntrusted) {
>+    flags |= NS_PRIV_EVENT_UNTRUSTED_PERMITTED;
>+  }
>+  listenerManager->AddEventListenerByType(aListener, aType, flags);
>+  return NS_OK;
>+}
>+
> #endif // nsEventListenerManager_h__
>diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp
>--- a/dom/base/nsGlobalWindow.cpp
>+++ b/dom/base/nsGlobalWindow.cpp
>@@ -7372,16 +7372,18 @@ nsGlobalWindow::RemoveEventListener(cons
> {
>   nsRefPtr<nsEventListenerManager> elm = GetListenerManager(false);
>   if (elm) {
>     elm->RemoveEventListener(aType, aListener, aUseCapture);
>   }
>   return NS_OK;
> }
> 
>+NS_IMPL_REMOVE_SYSTEM_EVENT_LISTENER(nsGlobalWindow)
>+
> NS_IMETHODIMP
> nsGlobalWindow::DispatchEvent(nsIDOMEvent* aEvent, bool* aRetVal)
> {
>   FORWARD_TO_INNER(DispatchEvent, (aEvent, aRetVal), NS_OK);
> 
>   if (!mDoc) {
>     return NS_ERROR_FAILURE;
>   }
>@@ -7425,16 +7427,42 @@ nsGlobalWindow::AddEventListener(const n
>   }
> 
>   nsEventListenerManager* manager = GetListenerManager(true);
>   NS_ENSURE_STATE(manager);
>   manager->AddEventListener(aType, aListener, aUseCapture, aWantsUntrusted);
>   return NS_OK;
> }
> 
>+NS_IMETHODIMP
>+nsGlobalWindow::AddSystemEventListener(const nsAString& aType,
>+                                       nsIDOMEventListener *aListener,
>+                                       bool aUseCapture,
>+                                       bool aWantsUntrusted,
>+                                       PRUint8 aOptionalArgc)
>+{
>+  NS_ASSERTION(!aWantsUntrusted || aOptionalArgc > 1,
>+               "Won't check if this is chrome, you want to set "
>+               "aWantsUntrusted to false or make the aWantsUntrusted "
>+               "explicit by making optional_argc non-zero.");
>+
>+  if (IsOuterWindow() && mInnerWindow &&
>+      !nsContentUtils::CanCallerAccess(mInnerWindow)) {
>+    return NS_ERROR_DOM_SECURITY_ERR;
>+  }
>+
>+  if (!aWantsUntrusted &&
>+      (aOptionalArgc < 2 && !nsContentUtils::IsChromeDoc(mDoc))) {
>+    aWantsUntrusted = true;
>+  }
>+
>+  return NS_AddSystemEventListener(this, aType, aListener, aUseCapture,
>+                                   aWantsUntrusted);
>+}
>+
> nsEventListenerManager*
> nsGlobalWindow::GetListenerManager(bool aCreateIfNotFound)
> {
>   FORWARD_TO_INNER_CREATE(GetListenerManager, (aCreateIfNotFound), nsnull);
> 
>   if (!mListenerManager && aCreateIfNotFound) {
>     mListenerManager =
>       new nsEventListenerManager(static_cast<nsIDOMEventTarget*>(this));
>diff --git a/dom/base/nsWindowRoot.cpp b/dom/base/nsWindowRoot.cpp
>--- a/dom/base/nsWindowRoot.cpp
>+++ b/dom/base/nsWindowRoot.cpp
>@@ -106,16 +106,18 @@ nsWindowRoot::RemoveEventListener(const 
> {
>   nsRefPtr<nsEventListenerManager> elm = GetListenerManager(false);
>   if (elm) {
>     elm->RemoveEventListener(aType, aListener, aUseCapture);
>   }
>   return NS_OK;
> }
> 
>+NS_IMPL_REMOVE_SYSTEM_EVENT_LISTENER(nsWindowRoot)
>+
> NS_IMETHODIMP
> nsWindowRoot::DispatchEvent(nsIDOMEvent* aEvt, bool *aRetVal)
> {
>   nsEventStatus status = nsEventStatus_eIgnore;
>   nsresult rv =  nsEventDispatcher::DispatchDOMEvent(
>     static_cast<nsIDOMEventTarget*>(this), nsnull, aEvt, nsnull, &status);
>   *aRetVal = (status != nsEventStatus_eConsumeNoDefault);
>   return rv;
>@@ -144,16 +146,32 @@ nsWindowRoot::AddEventListener(const nsA
>                "explicit by making optional_argc non-zero.");
> 
>   nsEventListenerManager* elm = GetListenerManager(true);
>   NS_ENSURE_STATE(elm);
>   elm->AddEventListener(aType, aListener, aUseCapture, aWantsUntrusted);
>   return NS_OK;
> }
> 
>+NS_IMETHODIMP
>+nsWindowRoot::AddSystemEventListener(const nsAString& aType,
>+                                     nsIDOMEventListener *aListener,
>+                                     bool aUseCapture,
>+                                     bool aWantsUntrusted,
>+                                     PRUint8 aOptionalArgc)
>+{
>+  NS_ASSERTION(!aWantsUntrusted || aOptionalArgc > 1,
>+               "Won't check if this is chrome, you want to set "
>+               "aWantsUntrusted to false or make the aWantsUntrusted "
>+               "explicit by making optional_argc non-zero.");
>+
>+  return NS_AddSystemEventListener(this, aType, aListener, aUseCapture,
>+                                   aWantsUntrusted);
>+}
>+
> nsEventListenerManager*
> nsWindowRoot::GetListenerManager(bool aCreateIfNotFound)
> {
>   if (!mListenerManager && aCreateIfNotFound) {
>     mListenerManager =
>       new nsEventListenerManager(static_cast<nsIDOMEventTarget*>(this));
>   }
> 
>diff --git a/dom/interfaces/events/nsIDOMEventTarget.idl b/dom/interfaces/events/nsIDOMEventTarget.idl
>--- a/dom/interfaces/events/nsIDOMEventTarget.idl
>+++ b/dom/interfaces/events/nsIDOMEventTarget.idl
>@@ -64,17 +64,17 @@ class nsEventListenerManager;
> [ptr] native nsEventStatusPtr(nsEventStatus);
> [ptr] native JSContextPtr(JSContext);
> [ptr] native nsEventListenerManagerPtr(nsEventListenerManager);
> 
> interface nsIScriptContext;
> interface nsIDOMEventListener;
> interface nsIDOMEvent;
> 
>-[scriptable, builtinclass, uuid(1797d5a4-b12a-428d-9eef-a0e13839728c)]
>+[scriptable, builtinclass, uuid(8e375931-298d-4d0a-9cb4-5668f0cdc5a8)]
> interface nsIDOMEventTarget : nsISupports
> {
>   /**
>    * This method allows the registration of event listeners on the event target.
>    * If an EventListener is added to an EventTarget while it is processing an
>    * event, it will not be triggered by the current actions but may be 
>    * triggered during a later stage of event flow, such as the bubbling phase.
>    * 
>@@ -101,32 +101,70 @@ interface nsIDOMEventTarget : nsISupport
>    *                         listener will receive events whether or not
>    *                         they're trusted
>    */
>   [optional_argc] void addEventListener(in DOMString type,
>                                         in nsIDOMEventListener listener,
>                                         [optional] in boolean useCapture,
>                                         [optional] in boolean wantsUntrusted);
> 
>+  /**
>+   * addSystemEventListener() adds an event listener of aType to the system
>+   * group.  Typically, core code should use system group for listening to
>+   * content (i.e., non-chrome) element's events.  If core code uses
>+   * nsIDOMEventTarget::AddEventListener for a content node, it means
>+   * that the listener cannot listen the event when web content calls
>+   * stopPropagation() of the event.
>+   *
>+   * @param aType            An event name you're going to handle.
>+   * @param aListener        An event listener.
>+   * @param aUseCapture      TRUE if you want to listen the event in capturing
>+   *                         phase.  Otherwise, FALSE.
>+   * @param aWantsUntrusted  TRUE if you want to handle untrusted events.
>+   *                         Otherwise, FALSE.
>+   * @return                 NS_OK if succeed.  Otherwise, NS_ERROR_*.
>+   */
>+  [noscript, optional_argc] void addSystemEventListener(
>+                                   in DOMString type,
>+                                   in nsIDOMEventListener listener,
>+                                   [optional] in boolean aUseCapture,
>+                                   [optional] in boolean aWantsUntrusted);
>+
> %{C++
>   // non-virtual so it won't affect the vtable
>   nsresult AddEventListener(const nsAString& aType,
>                             nsIDOMEventListener* aListener,
>                             bool aUseCapture)
>   {
>     return AddEventListener(aType, aListener, aUseCapture, PR_FALSE, 1);
>   }
>   // non-virtual so it won't affect the vtable
>   nsresult AddEventListener(const nsAString& aType,
>                             nsIDOMEventListener* aListener,
>                             bool aUseCapture,
>                             bool aWantsUntrusted)
>   {
>     return AddEventListener(aType, aListener, aUseCapture, aWantsUntrusted, 2);
>   }
>+  // non-virtual so it won't affect the vtable
>+  nsresult AddSystemEventListener(const nsAString& aType,
>+                                  nsIDOMEventListener* aListener,
>+                                  bool aUseCapture)
>+  {
>+    return AddSystemEventListener(aType, aListener, aUseCapture, PR_FALSE, 1);
>+  }
>+  // non-virtual so it won't affect the vtable
>+  nsresult AddSystemEventListener(const nsAString& aType,
>+                                  nsIDOMEventListener* aListener,
>+                                  bool aUseCapture,
>+                                  bool aWantsUntrusted)
>+  {
>+    return AddSystemEventListener(aType, aListener, aUseCapture,
>+                                  aWantsUntrusted, 2);
>+  }
> %}
> 
>   /**
>    * This method allows the removal of event listeners from the event 
>    * target. If an EventListener is removed from an EventTarget while it 
>    * is processing an event, it will not be triggered by the current actions. 
>    * EventListeners can never be invoked after being removed.
>    * Calling removeEventListener with arguments which do not identify any 
>@@ -144,16 +182,25 @@ interface nsIDOMEventTarget : nsISupport
>    *                     not affect a non-capturing version of the same 
>    *                     listener, and vice versa.
>    */
>   void                     removeEventListener(in DOMString type,
>                                                in nsIDOMEventListener listener,
>                                                [optional] in boolean useCapture);
> 
>   /**
>+   * removeSystemEventListener() should be used if you have used
>+   * addSystemEventListener().
>+   */
>+  [noscript] void          removeSystemEventListener(
>+                             in DOMString type,
>+                             in nsIDOMEventListener listener,
>+                             [optional] in boolean aUseCapture);
>+
>+  /**
>    * This method allows the dispatch of events into the implementations 
>    * event model. Events dispatched in this manner will have the same 
>    * capturing and bubbling behavior as events dispatched directly by the 
>    * implementation. The target of the event is the EventTarget on which 
>    * dispatchEvent is called.
>    *
>    * @param   evt Specifies the event type, behavior, and contextual 
>    *              information to be used in processing the event.
>@@ -273,9 +320,25 @@ interface nsIDOMEventTarget : nsISupport
> typedef nsIDOMEventTarget nsPIDOMEventTarget;
> 
> #define NS_IMPL_DOMTARGET_DEFAULTS(_class) \
> nsPIDOMEventTarget* _class::GetTargetForDOMEvent() { return this; } \
> nsPIDOMEventTarget* _class::GetTargetForEventTargetChain() { return this; } \
> nsresult _class::WillHandleEvent(nsEventChainPostVisitor& aVisitor) { return NS_OK; } \
> JSContext* _class::GetJSContextForEventHandlers() { return nsnull; }
> 
>+#define NS_IMPL_REMOVE_SYSTEM_EVENT_LISTENER(aClass) \
>+NS_IMETHODIMP \
>+aClass::RemoveSystemEventListener(const nsAString& aType, \
>+                                  nsIDOMEventListener *aListener, \
>+                                  bool aUseCapture) \
>+{ \
>+  nsEventListenerManager* listenerManager = GetListenerManager(false); \
>+  if (!listenerManager) { \
>+    return NS_OK; \
>+  } \
>+  PRUint32 flags = NS_EVENT_FLAG_SYSTEM_EVENT; \
>+  flags |= aUseCapture ? NS_EVENT_FLAG_CAPTURE : NS_EVENT_FLAG_BUBBLE; \
>+  listenerManager->RemoveEventListenerByType(aListener, aType, flags); \
>+  return NS_OK; \
>+}
>+
> %}
>diff --git a/layout/xul/base/src/nsSliderFrame.cpp b/layout/xul/base/src/nsSliderFrame.cpp
>--- a/layout/xul/base/src/nsSliderFrame.cpp
>+++ b/layout/xul/base/src/nsSliderFrame.cpp
>@@ -49,16 +49,17 @@
> #include "nsPresContext.h"
> #include "nsIContent.h"
> #include "nsCOMPtr.h"
> #include "nsINameSpaceManager.h"
> #include "nsGkAtoms.h"
> #include "nsHTMLParts.h"
> #include "nsIPresShell.h"
> #include "nsCSSRendering.h"
>+#include "nsEventListenerManager.h"
> #include "nsIDOMEventTarget.h"
> #include "nsIDOMMouseEvent.h"
> #include "nsIDocument.h"
> #include "nsScrollbarButtonFrame.h"
> #include "nsISliderListener.h"
> #include "nsIScrollbarMediator.h"
> #include "nsScrollbarFrame.h"
> #include "nsRepeatService.h"
>@@ -969,34 +970,35 @@ nsSliderFrame::isDraggingThumb()
> void
> nsSliderFrame::AddListener()
> {
>   if (!mMediator) {
>     mMediator = new nsSliderMediator(this);
>   }
> 
>   nsIFrame* thumbFrame = mFrames.FirstChild();
>-  if (thumbFrame) {
>-    thumbFrame->GetContent()->
>-      AddEventListener(NS_LITERAL_STRING("mousedown"), mMediator, false,
>-                       false);
>+  if (!thumbFrame) {
>+    return;
>   }
>+  thumbFrame->GetContent()->
>+    AddSystemEventListener(NS_LITERAL_STRING("mousedown"), mMediator,
>+                           false, false);
> }
> 
> void
> nsSliderFrame::RemoveListener()
> {
>   NS_ASSERTION(mMediator, "No listener was ever added!!");
> 
>   nsIFrame* thumbFrame = mFrames.FirstChild();
>   if (!thumbFrame)
>     return;
> 
>   thumbFrame->GetContent()->
>-    RemoveEventListener(NS_LITERAL_STRING("mousedown"), mMediator, false);
>+    RemoveSystemEventListener(NS_LITERAL_STRING("mousedown"), mMediator, false);
> }
> 
> NS_IMETHODIMP
> nsSliderFrame::HandlePress(nsPresContext* aPresContext,
>                            nsGUIEvent*     aEvent,
>                            nsEventStatus*  aEventStatus)
> {
> #ifdef XP_MACOSX
>diff --git a/layout/xul/test/Makefile.in b/layout/xul/test/Makefile.in
>--- a/layout/xul/test/Makefile.in
>+++ b/layout/xul/test/Makefile.in
>@@ -49,15 +49,16 @@ _TEST_FILES =\
> 		test_bug394800.xhtml \
> 		test_bug563416.html \
> 		$(NULL)
> 
> _CHROME_FILES = \
> 		test_bug372685.xul \
> 		test_bug398982-1.xul \
> 		test_bug398982-2.xul \
>+		test_bug703150.xul \
> 		$(NULL)
> 
> libs:: $(_TEST_FILES)
> 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
> 
> libs:: $(_CHROME_FILES)
> 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/chrome/$(relativesrcdir)
>diff --git a/layout/xul/test/test_bug703150.xul b/layout/xul/test/test_bug703150.xul
>new file mode 100644
>--- /dev/null
>+++ b/layout/xul/test/test_bug703150.xul
>@@ -0,0 +1,69 @@
>+<?xml version="1.0"?>
>+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
>+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
>+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>+        xmlns:html="http://www.w3.org/1999/xhtml"
>+        title="Test for Bug 703150">
>+<!--
>+https://bugzilla.mozilla.org/show_bug.cgi?id=703150
>+-->
>+
>+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
>+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
>+
>+<scrollbar id="scrollbar" curpos="0" maxpos="500"/>
>+
>+<script class="testbody" type="application/javascript">
>+<![CDATA[
>+
>+var scrollbar = document.getElementById("scrollbar");
>+var scrollbarThumb =
>+  document.getAnonymousElementByAttribute(scrollbar, "sbattr",
>+                                          "scrollbar-thumb");
>+
>+function doTest()
>+{
>+  function mousedownHandler(aEvent)
>+  {
>+    aEvent.stopPropagation();
>+  }
>+  window.addEventListener("mousedown", mousedownHandler, true);
>+
>+  // Wait for finishing reflow...
>+  SimpleTest.executeSoon(function () {
>+    synthesizeMouseAtCenter(scrollbarThumb, { type: "mousedown" });
>+
>+    is(scrollbar.getAttribute("curpos"), 0,
>+       "scrollbar thumb has been moved already");
>+
>+    synthesizeMouseAtCenter(scrollbar, { type: "mousemove" });
>+
>+    ok(scrollbar.getAttribute("curpos") > 0,
>+       "scrollbar thumb hasn't been dragged");
>+
>+    synthesizeMouseAtCenter(scrollbarThumb, { type: "mouseup" });
>+
>+    window.removeEventListener("mousedown", mousedownHandler, true);
>+
>+    SimpleTest.finish();
>+  });
>+}
>+
>+SimpleTest.waitForExplicitFinish();
>+
>+]]>
>+</script>
>+
>+<body  id="html_body" xmlns="http://www.w3.org/1999/xhtml">
>+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=703150">Mozilla Bug 703150</a>
>+<p id="display"></p>
>+
>+<pre id="test">
>+</pre>
>+<script>
>+addLoadEvent(doTest);
>+</script>
>+</body>
>+
>+
>+</window>

Comment 11

[Johnny Stenback (:jst)]

Just to clarify, this patch does add to nsIDOMEventTarget's vtable (by adding addSystemEventListener in the IDL). That means we should probably update the UUID of everything that inherits that, which is this list:

http://dxr.mozilla.org/mozilla/search.cgi?tree=mozilla-central&derived=nsIDOMEventTarget

Or am I missing something?

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

jst:

Even if the inherited classes don't override the new methods, the vtables will be changed?

Comment 13

[Johnny Stenback (:jst)]

Yes, adding a virtual method to a class (interface) will shift everything in the vtables in every class that inherits from the changed class.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

(In reply to Johnny Stenback (:jst, [e-mail]) from comment #13)
> Yes, adding a virtual method to a class (interface) will shift everything in
> the vtables in every class that inherits from the changed class.

I see. I'll change them.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Attachment: Patch

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 576686 [details] [diff] [review]
Patch

Sorry, I should have noticed the need for uuid updates

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b8cf3f10e4d3

no problem, the double review is helpful :-)

Comment 18

[:Mook]

Quick question: after this patch lands, how should one cancel moving of the scrollbar thumb?  (Assume the caller is chrome script, please.  Though I guess in either case this might be dev-doc-needed.)

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/specialpowersAPI.js#576

nsIEventListenerService has addSystemEventListener(). It adds the script's event listener to the system event group. Then, the chrome script can stopPropagation() in the system event group (scrollbar thumb listens to the event at bubble phase in system group).

Comment 20

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/mozilla-central/rev/b8cf3f10e4d3