From dc253ce953f0470e99a3493b64dea740552d1b28 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Thu, 30 Nov 2023 18:56:26 +0100 Subject: [PATCH] Issue #2362 - Fix click handling according to the spec. This removes some hackery surrounding preventing content clicks, and in general handles auxclick as it should, firing that event on secondary buttons (wheel/right on default setup for right-handed mouse). --- dom/base/nsDocument.cpp | 10 +++- dom/base/nsGlobalWindow.cpp | 5 +- dom/events/Event.cpp | 71 ++++++++++++++---------- dom/events/EventStateManager.cpp | 20 +++---- dom/html/HTMLInputElement.cpp | 28 ++-------- dom/html/HTMLTextAreaElement.cpp | 18 ------ editor/libeditor/EditorEventListener.cpp | 15 +++++ modules/libpref/init/all.js | 2 +- 8 files changed, 83 insertions(+), 86 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 153b070649..4d3fc70b83 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -7318,9 +7318,13 @@ nsresult nsDocument::GetEventTargetParent(EventChainPreVisitor& aVisitor) { aVisitor.mCanHandle = true; - // FIXME! This is a hack to make middle mouse paste working also in Editor. - // Bug 329119 - aVisitor.mForceContentDispatch = true; + // Middle/right click shouldn't dispatch click event, use auxclick to instead. + Element* docElement = GetRootElement(); + if (docElement && docElement->IsXULElement()) { + // FIXME! This is a hack to make middle mouse paste working also in Editor. + // Bug 329119 + aVisitor.mForceContentDispatch = true; + } // Load events must not propagate to |window| object, see bug 335251. if (aVisitor.mEvent->mMessage != eLoad) { diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 50f115ba36..295c498965 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -3544,7 +3544,10 @@ nsGlobalWindow::GetEventTargetParent(EventChainPreVisitor& aVisitor) EventMessage msg = aVisitor.mEvent->mMessage; aVisitor.mCanHandle = true; - aVisitor.mForceContentDispatch = true; //FIXME! Bug 329119 + // Middle/right click shouldn't dispatch click event, use auxclick to instead. + if (mDoc->IsXULDocument()) { + aVisitor.mForceContentDispatch = true; //FIXME! Bug 329119 + } if (msg == eResize && aVisitor.mEvent->IsTrusted()) { // QIing to window so that we can keep the old behavior also in case // a child window is handling resize. diff --git a/dom/events/Event.cpp b/dom/events/Event.cpp index 31a39a5609..f9a4208126 100755 --- a/dom/events/Event.cpp +++ b/dom/events/Event.cpp @@ -812,37 +812,48 @@ Event::GetEventPopupControlState(WidgetEvent* aEvent, nsIDOMEvent* aDOMEvent) } break; case eMouseEventClass: - if (aEvent->IsTrusted() && - aEvent->AsMouseEvent()->button == WidgetMouseEvent::eLeftButton) { - switch(aEvent->mMessage) { - case eMouseUp: - if (PopupAllowedForEvent("mouseup")) { + if (aEvent->IsTrusted()) { + if(aEvent->AsMouseEvent()->button == WidgetMouseEvent::eLeftButton) { + switch(aEvent->mMessage) { + case eMouseUp: + if (PopupAllowedForEvent("mouseup")) { + abuse = openControlled; + } + break; + case eMouseDown: + if (PopupAllowedForEvent("mousedown")) { + abuse = openControlled; + } + break; + case eMouseClick: + /* Click events get special treatment because of their + historical status as a more legitimate event handler. If + click popups are enabled in the prefs, clear the popup + status completely. */ + if (PopupAllowedForEvent("click")) { + abuse = openAllowed; + } + break; + case eMouseDoubleClick: + if (PopupAllowedForEvent("dblclick")) { + abuse = openControlled; + } + break; + default: + break; + } + } else if (aEvent->mMessage == eMouseAuxClick) { + // Not eLeftButton + // There's not a strong reason to ignore other events (eg eMouseUp) + // for non-primary clicks as far as we know, so we could add them if + // it becomes a compat issue + if (PopupAllowedForEvent("auxclick")) { abuse = openControlled; - } - break; - case eMouseDown: - if (PopupAllowedForEvent("mousedown")) { - abuse = openControlled; - } - break; - case eMouseClick: - /* Click events get special treatment because of their - historical status as a more legitimate event handler. If - click popups are enabled in the prefs, clear the popup - status completely. */ - if (PopupAllowedForEvent("click")) { - abuse = openAllowed; - } - break; - case eMouseDoubleClick: - if (PopupAllowedForEvent("dblclick")) { - abuse = openControlled; - } - break; - default: - break; - } - } + } else { + abuse = openOverridden; + } + } + } // IsTrusted() break; case ePointerEventClass: if (aEvent->IsTrusted() && diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp index a3c3f5e0c8..dd753a97f0 100644 --- a/dom/events/EventStateManager.cpp +++ b/dom/events/EventStateManager.cpp @@ -4682,6 +4682,7 @@ EventStateManager::InitAndDispatchClickEvent(WidgetMouseEvent* aMouseUpEvent, event.buttons = aMouseUpEvent->buttons; event.mTime = aMouseUpEvent->mTime; event.mTimeStamp = aMouseUpEvent->mTimeStamp; + event.mFlags.mOnlyChromeDispatch = aNoContentDispatch; event.mFlags.mNoContentDispatch = aNoContentDispatch; event.button = aMouseUpEvent->button; event.inputSource = aMouseUpEvent->inputSource; @@ -4764,8 +4765,16 @@ EventStateManager::DispatchClickEvents(nsIPresShell* aPresShell, return ret; } + // Fire auxclick even if necessary. + if (fireAuxClick && aClickTarget && aClickTarget->IsInComposedDoc()) { + ret = InitAndDispatchClickEvent(aMouseUpEvent, aStatus, eMouseAuxClick, + aPresShell, aClickTarget, currentTarget, + false); + NS_WARNING_ASSERTION(NS_SUCCEEDED(ret), "Failed to dispatch eMouseAuxClick"); + } + // Fire double click event if click count is 2. - if (aMouseUpEvent->mClickCount == 2 && + if (aMouseUpEvent->mClickCount == 2 && !fireAuxClick && aClickTarget && aClickTarget->IsInComposedDoc()) { ret = InitAndDispatchClickEvent(aMouseUpEvent, aStatus, eMouseDoubleClick, aPresShell, aClickTarget, currentTarget, @@ -4775,15 +4784,6 @@ EventStateManager::DispatchClickEvents(nsIPresShell* aPresShell, } } - // Fire auxclick even if necessary. - if (fireAuxClick && - aClickTarget && aClickTarget->IsInComposedDoc()) { - ret = InitAndDispatchClickEvent(aMouseUpEvent, aStatus, eMouseAuxClick, - aPresShell, aClickTarget, currentTarget, - false); - NS_WARNING_ASSERTION(NS_SUCCEEDED(ret), "Failed to dispatch eMouseAuxClick"); - } - return ret; } diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 2f4ee84a49..c5849de49e 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -141,16 +141,15 @@ namespace dom { // First bits are needed for the control type. #define NS_OUTER_ACTIVATE_EVENT (1 << 9) #define NS_ORIGINAL_CHECKED_VALUE (1 << 10) -#define NS_NO_CONTENT_DISPATCH (1 << 11) +// (1 << 11 is unused) #define NS_ORIGINAL_INDETERMINATE_VALUE (1 << 12) #define NS_PRE_HANDLE_BLUR_EVENT (1 << 13) #define NS_PRE_HANDLE_INPUT_EVENT (1 << 14) #define NS_IN_SUBMIT_CLICK (1 << 15) -#define NS_CONTROL_TYPE(bits) \ - ((bits) & ~(NS_OUTER_ACTIVATE_EVENT | NS_ORIGINAL_CHECKED_VALUE | \ - NS_NO_CONTENT_DISPATCH | NS_ORIGINAL_INDETERMINATE_VALUE | \ - NS_PRE_HANDLE_BLUR_EVENT | NS_PRE_HANDLE_INPUT_EVENT | \ - NS_IN_SUBMIT_CLICK)) +#define NS_CONTROL_TYPE(bits) \ + ((bits) & ~(NS_OUTER_ACTIVATE_EVENT | NS_ORIGINAL_CHECKED_VALUE | \ + NS_ORIGINAL_INDETERMINATE_VALUE | NS_PRE_HANDLE_BLUR_EVENT | \ + NS_PRE_HANDLE_INPUT_EVENT | NS_IN_SUBMIT_CLICK)) // whether textfields should be selected once focused: // -1: no, 1: yes, 0: uninitialized @@ -3823,19 +3822,6 @@ HTMLInputElement::GetEventTargetParent(EventChainPreVisitor& aVisitor) aVisitor.mItemFlags |= NS_ORIGINAL_CHECKED_VALUE; } - // If mNoContentDispatch is true we will not allow content to handle - // this event. But to allow middle mouse button paste to work we must allow - // middle clicks to go to text fields anyway. - if (aVisitor.mEvent->mFlags.mNoContentDispatch) { - aVisitor.mItemFlags |= NS_NO_CONTENT_DISPATCH; - } - if (IsSingleLineTextControl(false) && - aVisitor.mEvent->mMessage == eMouseClick && - aVisitor.mEvent->AsMouseEvent()->button == - WidgetMouseEvent::eMiddleButton) { - aVisitor.mEvent->mFlags.mNoContentDispatch = false; - } - // We must cache type because mType may change during JS event (bug 2369) aVisitor.mItemFlags |= mType; @@ -4363,7 +4349,6 @@ HTMLInputElement::PostHandleEvent(EventChainPostVisitor& aVisitor) bool outerActivateEvent = !!(aVisitor.mItemFlags & NS_OUTER_ACTIVATE_EVENT); bool originalCheckedValue = !!(aVisitor.mItemFlags & NS_ORIGINAL_CHECKED_VALUE); - bool noContentDispatch = !!(aVisitor.mItemFlags & NS_NO_CONTENT_DISPATCH); uint8_t oldType = NS_CONTROL_TYPE(aVisitor.mItemFlags); // Ideally we would make the default action for click and space just dispatch @@ -4415,9 +4400,6 @@ HTMLInputElement::PostHandleEvent(EventChainPostVisitor& aVisitor) } } - // Reset the flag for other content besides this text field - aVisitor.mEvent->mFlags.mNoContentDispatch = noContentDispatch; - // now check to see if the event was "cancelled" if (mCheckedIsToggled && outerActivateEvent) { if (aVisitor.mEventStatus == nsEventStatus_eConsumeNoDefault) { diff --git a/dom/html/HTMLTextAreaElement.cpp b/dom/html/HTMLTextAreaElement.cpp index 0514a26607..9162a7356d 100644 --- a/dom/html/HTMLTextAreaElement.cpp +++ b/dom/html/HTMLTextAreaElement.cpp @@ -43,8 +43,6 @@ static NS_DEFINE_CID(kXULControllersCID, NS_XULCONTROLLERS_CID); -#define NS_NO_CONTENT_DISPATCH (1 << 0) - NS_IMPL_NS_NEW_HTML_ELEMENT_CHECK_PARSER(TextArea) namespace mozilla { @@ -521,18 +519,6 @@ HTMLTextAreaElement::GetEventTargetParent(EventChainPreVisitor& aVisitor) mHandlingSelect = true; } - // If noContentDispatch is true we will not allow content to handle - // this event. But to allow middle mouse button paste to work we must allow - // middle clicks to go to text fields anyway. - if (aVisitor.mEvent->mFlags.mNoContentDispatch) { - aVisitor.mItemFlags |= NS_NO_CONTENT_DISPATCH; - } - if (aVisitor.mEvent->mMessage == eMouseClick && - aVisitor.mEvent->AsMouseEvent()->button == - WidgetMouseEvent::eMiddleButton) { - aVisitor.mEvent->mFlags.mNoContentDispatch = false; - } - if (aVisitor.mEvent->mMessage == eBlur) { // Set mWantsPreHandleEvent and fire change event in PreHandleEvent to // prevent it breaks event target chain creation. @@ -596,10 +582,6 @@ HTMLTextAreaElement::PostHandleEvent(EventChainPostVisitor& aVisitor) UpdateState(true); } - // Reset the flag for other content besides this text field - aVisitor.mEvent->mFlags.mNoContentDispatch = - ((aVisitor.mItemFlags & NS_NO_CONTENT_DISPATCH) != 0); - return NS_OK; } diff --git a/editor/libeditor/EditorEventListener.cpp b/editor/libeditor/EditorEventListener.cpp index d809d8d8cd..cba58ed7bc 100644 --- a/editor/libeditor/EditorEventListener.cpp +++ b/editor/libeditor/EditorEventListener.cpp @@ -189,6 +189,9 @@ EditorEventListener::InstallToEditor() elmP->AddEventListenerByType(this, NS_LITERAL_STRING("click"), TrustedEventsAtCapture()); + elmP->AddEventListenerByType(this, + NS_LITERAL_STRING("auxclick"), + TrustedEventsAtSystemGroupCapture()); // Focus event doesn't bubble so adding the listener to capturing phase. // Make sure this works after bug 235441 gets fixed. elmP->AddEventListenerByType(this, @@ -282,6 +285,9 @@ EditorEventListener::UninstallFromEditor() elmP->RemoveEventListenerByType(this, NS_LITERAL_STRING("click"), TrustedEventsAtCapture()); + elmP->RemoveEventListenerByType(this, + NS_LITERAL_STRING("auxclick"), + TrustedEventsAtSystemGroupCapture()); elmP->RemoveEventListenerByType(this, NS_LITERAL_STRING("blur"), TrustedEventsAtCapture()); @@ -447,6 +453,15 @@ EditorEventListener::HandleEvent(nsIDOMEvent* aEvent) } // click case eMouseClick: { + WidgetMouseEvent* widgetMouseEvent = internalEvent->AsMouseEvent(); + // Don't handle non-primary click events + if (widgetMouseEvent->button != WidgetMouseEvent::eLeftButton) { + return NS_OK; + } + [[fallthrough]]; + } + // auxclick + case eMouseAuxClick: { nsCOMPtr mouseEvent = do_QueryInterface(aEvent); NS_ENSURE_TRUE(mouseEvent, NS_OK); // If the preceding mousedown event or mouseup event was consumed, diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 344bb532b2..9adfe7cc17 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1156,7 +1156,7 @@ pref("dom.require_user_interaction_for_beforeunload", true); pref("dom.disable_open_during_load", false); pref("dom.popup_maximum", 20); -pref("dom.popup_allowed_events", "change click dblclick mouseup pointerup notificationclick reset submit touchend"); +pref("dom.popup_allowed_events", "change click dblclick auxclick mouseup pointerup notificationclick reset submit touchend"); pref("dom.disable_open_click_delay", 1000); pref("dom.storage.enabled", true);