From 062916b7a57e33a036dc4f3b7442153c4d2c5dac Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Mon, 9 Dec 2013 00:51:16 +0900 Subject: [PATCH] Bug 930374 part.1 Event.defaultPrevented should be false even if preventDefault() has been called by default action handler r=smaug --- content/events/src/moz.build | 1 + content/events/src/nsDOMEvent.cpp | 88 +++++++++++++++---- content/events/src/nsDOMEvent.h | 27 +++++- content/events/src/nsEventStateManager.cpp | 52 ++++++----- content/events/src/nsEventStateManager.h | 25 ++++-- content/events/test/chrome.ini | 1 + content/events/test/mochitest.ini | 1 + .../events/test/test_bug930374-chrome.html | 59 +++++++++++++ .../events/test/test_bug930374-content.html | 72 +++++++++++++++ dom/bindings/Bindings.conf | 1 + 10 files changed, 281 insertions(+), 46 deletions(-) create mode 100644 content/events/test/test_bug930374-chrome.html create mode 100644 content/events/test/test_bug930374-content.html diff --git a/content/events/src/moz.build b/content/events/src/moz.build index c4c306f01fd4..285a66459832 100644 --- a/content/events/src/moz.build +++ b/content/events/src/moz.build @@ -86,6 +86,7 @@ LOCAL_INCLUDES += [ '/dom/base', '/dom/settings', '/dom/src/storage', + '/js/xpconnect/wrappers', '/layout/generic', '/layout/xul', '/layout/xul/tree/', diff --git a/content/events/src/nsDOMEvent.cpp b/content/events/src/nsDOMEvent.cpp index 888f47354ec7..52369931bbf1 100644 --- a/content/events/src/nsDOMEvent.cpp +++ b/content/events/src/nsDOMEvent.cpp @@ -5,6 +5,7 @@ #include "base/basictypes.h" +#include "AccessCheck.h" #include "ipc/IPCMessageUtils.h" #include "nsCOMPtr.h" #include "nsError.h" @@ -33,6 +34,14 @@ using namespace mozilla; using namespace mozilla::dom; +namespace mozilla { +namespace dom { +namespace workers { +extern bool IsCurrentThreadRunningChromeWorker(); +} // namespace workers +} // namespace dom +} // namespace mozilla + static char *sPopupAllowedEvents; @@ -217,6 +226,14 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsDOMEvent) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END +bool +nsDOMEvent::IsChrome(JSContext* aCx) const +{ + return mIsMainThreadEvent ? + xpc::AccessCheck::isChrome(js::GetContextCompartment(aCx)) : + mozilla::dom::workers::IsCurrentThreadRunningChromeWorker(); +} + // nsIDOMEventInterface NS_METHOD nsDOMEvent::GetType(nsAString& aType) { @@ -436,25 +453,40 @@ nsDOMEvent::GetIsTrusted(bool *aIsTrusted) NS_IMETHODIMP nsDOMEvent::PreventDefault() { - if (mEvent->mFlags.mCancelable) { - mEvent->mFlags.mDefaultPrevented = true; + // This method is called only from C++ code which must handle default action + // of this event. So, pass true always. + PreventDefaultInternal(true); + return NS_OK; +} - // Need to set an extra flag for drag events. - if (mEvent->eventStructType == NS_DRAG_EVENT && IsTrusted()) { - nsCOMPtr node = do_QueryInterface(mEvent->currentTarget); - if (!node) { - nsCOMPtr win = do_QueryInterface(mEvent->currentTarget); - if (win) { - node = win->GetExtantDoc(); - } - } - if (node && !nsContentUtils::IsChromeDoc(node->OwnerDoc())) { - mEvent->mFlags.mDefaultPreventedByContent = true; - } - } +void +nsDOMEvent::PreventDefault(JSContext* aCx) +{ + MOZ_ASSERT(aCx, "JS context must be specified"); + + // Note that at handling default action, another event may be dispatched. + // Then, JS in content mey be call preventDefault() + // even in the event is in system event group. Therefore, don't refer + // mInSystemGroup here. + PreventDefaultInternal(IsChrome(aCx)); +} + +void +nsDOMEvent::PreventDefaultInternal(bool aCalledByDefaultHandler) +{ + if (!mEvent->mFlags.mCancelable) { + return; } - return NS_OK; + mEvent->mFlags.mDefaultPrevented = true; + + // Note that even if preventDefault() has already been called by chrome, + // a call of preventDefault() by content needs to overwrite + // mDefaultPreventedByContent to true because in such case, defaultPrevented + // must be true when web apps check it after they call preventDefault(). + if (!aCalledByDefaultHandler) { + mEvent->mFlags.mDefaultPreventedByContent = true; + } } void @@ -1143,6 +1175,24 @@ const char* nsDOMEvent::GetEventName(uint32_t aEventType) return nullptr; } +bool +nsDOMEvent::DefaultPrevented(JSContext* aCx) const +{ + MOZ_ASSERT(aCx, "JS context must be specified"); + + NS_ENSURE_TRUE(mEvent, false); + + // If preventDefault() has never been called, just return false. + if (!mEvent->mFlags.mDefaultPrevented) { + return false; + } + + // If preventDefault() has been called by content, return true. Otherwise, + // i.e., preventDefault() has been called by chrome, return true only when + // this is called by chrome. + return mEvent->mFlags.mDefaultPreventedByContent || IsChrome(aCx); +} + bool nsDOMEvent::GetPreventDefault() const { @@ -1151,6 +1201,9 @@ nsDOMEvent::GetPreventDefault() const doc->WarnOnceAbout(nsIDocument::eGetPreventDefault); } } + // GetPreventDefault() is legacy and Gecko specific method. Although, + // the result should be same as defaultPrevented, we don't need to break + // backward compatibility of legacy method. Let's behave traditionally. return DefaultPrevented(); } @@ -1166,6 +1219,9 @@ NS_IMETHODIMP nsDOMEvent::GetDefaultPrevented(bool* aReturn) { NS_ENSURE_ARG_POINTER(aReturn); + // This method must be called by only event handlers implemented by C++. + // Then, the handlers must handle default action. So, this method don't need + // to check if preventDefault() has been called by content or chrome. *aReturn = DefaultPrevented(); return NS_OK; } diff --git a/content/events/src/nsDOMEvent.h b/content/events/src/nsDOMEvent.h index ac234238b034..fbcd9341d498 100644 --- a/content/events/src/nsDOMEvent.h +++ b/content/events/src/nsDOMEvent.h @@ -153,9 +153,20 @@ public: // xpidl implementation // void PreventDefault(); + // You MUST NOT call PreventDefaultJ(JSContext*) from C++ code. A call of + // this method always sets Event.defaultPrevented true for web contents. + // If default action handler calls this, web applications meet wrong + // defaultPrevented value. + void PreventDefault(JSContext* aCx); + + // You MUST NOT call DefaultPrevented(JSContext*) from C++ code. This may + // return false even if PreventDefault() has been called. + // See comments in its implementation for the detail. + bool DefaultPrevented(JSContext* aCx) const; + bool DefaultPrevented() const { - return mEvent && mEvent->mFlags.mDefaultPrevented; + return mEvent->mFlags.mDefaultPrevented; } bool MultipleActionsPrevented() const @@ -190,6 +201,20 @@ protected: void SetEventType(const nsAString& aEventTypeArg); already_AddRefed GetTargetFromFrame(); + /** + * IsChrome() returns true if aCx is chrome context or the event is created + * in chrome's thread. Otherwise, false. + */ + bool IsChrome(JSContext* aCx) const; + + /** + * @param aCalledByDefaultHandler Should be true when this is called by + * C++ or Chrome. Otherwise, e.g., called + * by a call of Event.preventDefault() in + * content script, false. + */ + void PreventDefaultInternal(bool aCalledByDefaultHandler); + mozilla::WidgetEvent* mEvent; nsRefPtr mPresContext; nsCOMPtr mExplicitOriginalTarget; diff --git a/content/events/src/nsEventStateManager.cpp b/content/events/src/nsEventStateManager.cpp index ba2a5cfd5474..a658b51d3a7f 100644 --- a/content/events/src/nsEventStateManager.cpp +++ b/content/events/src/nsEventStateManager.cpp @@ -2610,10 +2610,13 @@ nsEventStateManager::DispatchLegacyMouseScrollEvents(nsIFrame* aTargetFrame, nsWeakFrame targetFrame(aTargetFrame); - nsEventStatus statusX = *aStatus; - nsEventStatus statusY = *aStatus; + MOZ_ASSERT(*aStatus != nsEventStatus_eConsumeNoDefault && + !aEvent->mFlags.mDefaultPrevented, + "If you make legacy events dispatched for default prevented wheel " + "event, you need to initialize stateX and stateY"); + EventState stateX, stateY; if (scrollDeltaY) { - SendLineScrollEvent(aTargetFrame, aEvent, &statusY, + SendLineScrollEvent(aTargetFrame, aEvent, stateY, scrollDeltaY, DELTA_DIRECTION_Y); if (!targetFrame.IsAlive()) { *aStatus = nsEventStatus_eConsumeNoDefault; @@ -2622,7 +2625,7 @@ nsEventStateManager::DispatchLegacyMouseScrollEvents(nsIFrame* aTargetFrame, } if (pixelDeltaY) { - SendPixelScrollEvent(aTargetFrame, aEvent, &statusY, + SendPixelScrollEvent(aTargetFrame, aEvent, stateY, pixelDeltaY, DELTA_DIRECTION_Y); if (!targetFrame.IsAlive()) { *aStatus = nsEventStatus_eConsumeNoDefault; @@ -2631,7 +2634,7 @@ nsEventStateManager::DispatchLegacyMouseScrollEvents(nsIFrame* aTargetFrame, } if (scrollDeltaX) { - SendLineScrollEvent(aTargetFrame, aEvent, &statusX, + SendLineScrollEvent(aTargetFrame, aEvent, stateX, scrollDeltaX, DELTA_DIRECTION_X); if (!targetFrame.IsAlive()) { *aStatus = nsEventStatus_eConsumeNoDefault; @@ -2640,7 +2643,7 @@ nsEventStateManager::DispatchLegacyMouseScrollEvents(nsIFrame* aTargetFrame, } if (pixelDeltaX) { - SendPixelScrollEvent(aTargetFrame, aEvent, &statusX, + SendPixelScrollEvent(aTargetFrame, aEvent, stateX, pixelDeltaX, DELTA_DIRECTION_X); if (!targetFrame.IsAlive()) { *aStatus = nsEventStatus_eConsumeNoDefault; @@ -2648,21 +2651,18 @@ nsEventStateManager::DispatchLegacyMouseScrollEvents(nsIFrame* aTargetFrame, } } - if (statusY == nsEventStatus_eConsumeNoDefault || - statusX == nsEventStatus_eConsumeNoDefault) { + if (stateY.mDefaultPrevented || stateX.mDefaultPrevented) { *aStatus = nsEventStatus_eConsumeNoDefault; - return; - } - if (statusY == nsEventStatus_eConsumeDoDefault || - statusX == nsEventStatus_eConsumeDoDefault) { - *aStatus = nsEventStatus_eConsumeDoDefault; + aEvent->mFlags.mDefaultPrevented = true; + aEvent->mFlags.mDefaultPreventedByContent |= + stateY.mDefaultPreventedByContent || stateX.mDefaultPreventedByContent; } } void nsEventStateManager::SendLineScrollEvent(nsIFrame* aTargetFrame, WidgetWheelEvent* aEvent, - nsEventStatus* aStatus, + EventState& aState, int32_t aDelta, DeltaDirection aDeltaDirection) { @@ -2678,9 +2678,8 @@ nsEventStateManager::SendLineScrollEvent(nsIFrame* aTargetFrame, WidgetMouseScrollEvent event(aEvent->mFlags.mIsTrusted, NS_MOUSE_SCROLL, aEvent->widget); - if (*aStatus == nsEventStatus_eConsumeNoDefault) { - event.mFlags.mDefaultPrevented = true; - } + event.mFlags.mDefaultPrevented = aState.mDefaultPrevented; + event.mFlags.mDefaultPreventedByContent = aState.mDefaultPreventedByContent; event.refPoint = aEvent->refPoint; event.widget = aEvent->widget; event.time = aEvent->time; @@ -2690,14 +2689,18 @@ nsEventStateManager::SendLineScrollEvent(nsIFrame* aTargetFrame, event.delta = aDelta; event.inputSource = aEvent->inputSource; + nsEventStatus status = nsEventStatus_eIgnore; nsEventDispatcher::Dispatch(targetContent, aTargetFrame->PresContext(), - &event, nullptr, aStatus); + &event, nullptr, &status); + aState.mDefaultPrevented = + event.mFlags.mDefaultPrevented || status == nsEventStatus_eConsumeNoDefault; + aState.mDefaultPreventedByContent = event.mFlags.mDefaultPreventedByContent; } void nsEventStateManager::SendPixelScrollEvent(nsIFrame* aTargetFrame, WidgetWheelEvent* aEvent, - nsEventStatus* aStatus, + EventState& aState, int32_t aPixelDelta, DeltaDirection aDeltaDirection) { @@ -2714,9 +2717,8 @@ nsEventStateManager::SendPixelScrollEvent(nsIFrame* aTargetFrame, WidgetMouseScrollEvent event(aEvent->mFlags.mIsTrusted, NS_MOUSE_PIXEL_SCROLL, aEvent->widget); - if (*aStatus == nsEventStatus_eConsumeNoDefault) { - event.mFlags.mDefaultPrevented = true; - } + event.mFlags.mDefaultPrevented = aState.mDefaultPrevented; + event.mFlags.mDefaultPreventedByContent = aState.mDefaultPreventedByContent; event.refPoint = aEvent->refPoint; event.widget = aEvent->widget; event.time = aEvent->time; @@ -2726,8 +2728,12 @@ nsEventStateManager::SendPixelScrollEvent(nsIFrame* aTargetFrame, event.delta = aPixelDelta; event.inputSource = aEvent->inputSource; + nsEventStatus status = nsEventStatus_eIgnore; nsEventDispatcher::Dispatch(targetContent, aTargetFrame->PresContext(), - &event, nullptr, aStatus); + &event, nullptr, &status); + aState.mDefaultPrevented = + event.mFlags.mDefaultPrevented || status == nsEventStatus_eConsumeNoDefault; + aState.mDefaultPreventedByContent = event.mFlags.mDefaultPreventedByContent; } nsIScrollableFrame* diff --git a/content/events/src/nsEventStateManager.h b/content/events/src/nsEventStateManager.h index 8128cb48af2f..34d366840aa9 100644 --- a/content/events/src/nsEventStateManager.h +++ b/content/events/src/nsEventStateManager.h @@ -481,6 +481,17 @@ protected: DELTA_DIRECTION_Y }; + struct MOZ_STACK_CLASS EventState + { + bool mDefaultPrevented; + bool mDefaultPreventedByContent; + + EventState() : + mDefaultPrevented(false), mDefaultPreventedByContent(false) + { + } + }; + /** * SendLineScrollEvent() dispatches a DOMMouseScroll event for the * WidgetWheelEvent. This method shouldn't be called for non-trusted @@ -488,14 +499,15 @@ protected: * * @param aTargetFrame The event target of wheel event. * @param aEvent The original Wheel event. - * @param aStatus The event status, must not be - * nsEventStatus_eConsumeNoDefault. + * @param aState The event which should be set to the dispatching + * event. This also returns the dispatched event + * state. * @param aDelta The delta value of the event. * @param aDeltaDirection The X/Y direction of dispatching event. */ void SendLineScrollEvent(nsIFrame* aTargetFrame, mozilla::WidgetWheelEvent* aEvent, - nsEventStatus* aStatus, + EventState& aState, int32_t aDelta, DeltaDirection aDeltaDirection); @@ -506,14 +518,15 @@ protected: * * @param aTargetFrame The event target of wheel event. * @param aEvent The original Wheel event. - * @param aStatus The event status, must not be - * nsEventStatus_eConsumeNoDefault. + * @param aState The event which should be set to the dispatching + * event. This also returns the dispatched event + * state. * @param aPixelDelta The delta value of the event. * @param aDeltaDirection The X/Y direction of dispatching event. */ void SendPixelScrollEvent(nsIFrame* aTargetFrame, mozilla::WidgetWheelEvent* aEvent, - nsEventStatus* aStatus, + EventState& aState, int32_t aPixelDelta, DeltaDirection aDeltaDirection); diff --git a/content/events/test/chrome.ini b/content/events/test/chrome.ini index 6ba24248a591..fd09c197e0de 100644 --- a/content/events/test/chrome.ini +++ b/content/events/test/chrome.ini @@ -15,4 +15,5 @@ support-files = [test_bug602962.xul] [test_bug617528.xul] [test_bug679494.xul] +[test_bug930374-chrome.html] [test_eventctors.xul] diff --git a/content/events/test/mochitest.ini b/content/events/test/mochitest.ini index 3f2caaf8ea27..53e20c50f22f 100644 --- a/content/events/test/mochitest.ini +++ b/content/events/test/mochitest.ini @@ -83,6 +83,7 @@ skip-if = true # Disabled due to timeouts. [test_bug847597.html] [test_bug855741.html] [test_bug864040.html] +[test_bug930374-content.html] skip-if = toolkit == "gonk" [test_clickevent_on_input.html] [test_continuous_wheel_events.html] diff --git a/content/events/test/test_bug930374-chrome.html b/content/events/test/test_bug930374-chrome.html new file mode 100644 index 000000000000..172ae5f821be --- /dev/null +++ b/content/events/test/test_bug930374-chrome.html @@ -0,0 +1,59 @@ + + + + + + Test for Bug 930374 + + + + + + +Mozilla Bug 930374 +
+ +
+ +
+  
+
+ + diff --git a/content/events/test/test_bug930374-content.html b/content/events/test/test_bug930374-content.html new file mode 100644 index 000000000000..73654bf04580 --- /dev/null +++ b/content/events/test/test_bug930374-content.html @@ -0,0 +1,72 @@ + + + + + + Test for Bug 930374 + + + + + +Mozilla Bug 930374 +
+ +
+ +
+  
+
+ + diff --git a/dom/bindings/Bindings.conf b/dom/bindings/Bindings.conf index c861da2999d0..807edf96001a 100644 --- a/dom/bindings/Bindings.conf +++ b/dom/bindings/Bindings.conf @@ -385,6 +385,7 @@ DOMInterfaces = { 'Event': { 'nativeType': 'nsDOMEvent', + 'implicitJSContext': [ 'defaultPrevented', 'preventDefault' ], }, 'EventTarget': {