This will be a whole set of patches. Will probably spread out landing them over a few days.

These are all ready to review, but I figured I should spread the load a bit. If anyone wants to review them, don't let me stop you :)

I'll try to review these today or tomorrow. If I don't, please ping me.

Comment on attachment 539090 [details] [diff] [review] Remove from nsImageMap and nsSliderFrame >+nsImageMap::HandleEvent(nsIDOMEvent* aEvent) > { >- return ChangeFocus(aEvent, PR_TRUE); >-} >+ nsAutoString eventType; >+ aEvent->GetType(eventType); >+ PRBool focus = eventType.EqualsLiteral("focus"); >+ NS_ABORT_IF_FASLSE(focus == !eventType.EqualsLiteral("blur"), >+ "Unexpected event type"); You didn't compile this patch? Or, as I assume, you just accidentally changed the patch after compiling it.

Comment on attachment 539095 [details] [diff] [review] Remove from mathml code >+nsMathMLmactionFrame::MouseListener::HandleEvent(nsIDOMEvent* aEvent) > { >- mOwner->MouseOver(); Could you add nsCOMPtr<nsIDOMMouseEvent> check here? >+ nsAutoString eventType; >+ aEvent->GetType(eventType); >+ if (eventType.EqualsLiteral("mouseover")) { >+ mOwner->MouseOver(); >+ } >+ else if (eventType.EqualsLiteral("mouseclick")) { >+ mOwner->MouseClick(); >+ } >+ else if (eventType.EqualsLiteral("mouseout")) { >+ mOwner->MouseOut(); >+ } >+ else { >+ NS_ABORT(); >+ } if (expr) { stmt; } else if (expr) { stmt; }

Comment on attachment 539096 [details] [diff] [review] Remove from nsDocumentViewer > nsDocViewerFocusListener::HandleEvent(nsIDOMEvent* aEvent) > { >+ if(!mDocViewer) >+ return NS_ERROR_FAILURE; if (expr) { stmt; } Or, NS_ENSURE_STATE(mDocViewer); >+ >+ nsCOMPtr<nsIPresShell> shell; >+ nsresult rv = mDocViewer->GetPresShell(getter_AddRefs(shell)); >+ NS_ENSURE_SUCCESS(rv, rv); No need for NS_ENSURE_SUCCESS >+ NS_ENSURE_TRUE(shell, NS_ERROR_FAILURE); >+ nsAutoString eventType; >+ aEvent->GetType(eventType); >+ if (eventType.EqualsLiteral("focus")) { >+ // If selection was disabled, re-enable it. >+ if(selectionStatus == nsISelectionController::SELECTION_DISABLED || >+ selectionStatus == nsISelectionController::SELECTION_HIDDEN) >+ { >+ selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON); >+ selCon->RepaintSelection(nsISelectionController::SELECTION_NORMAL); >+ } >+ } >+ else { if (expr) { stmt; } else { stmt; }

Comment on attachment 539097 [details] [diff] [review] Remove from nsListControlFrame >- // mouse event listeners (both might destroy |this|) > nsresult MouseDown(nsIDOMEvent* aMouseEvent); > nsresult MouseUp(nsIDOMEvent* aMouseEvent); >- >- // mouse motion listeners > nsresult MouseMove(nsIDOMEvent* aMouseEvent); > nsresult DragMove(nsIDOMEvent* aMouseEvent); >- >- // key listener (might destroy |this|) > nsresult KeyPress(nsIDOMEvent* aKeyEvent); Don't remove the "might destroy |this|" comments.

Comment on attachment 539098 [details] [diff] [review] Remove from nsComboboxControlFrame >+ NS_IMETHOD HandleEvent(nsIDOMEvent*) > { > mComboBox->ShowDropDown(!mComboBox->IsDroppedDown()); > return NS_OK; > } Could you add nsIDOMMouseEvent check here. I wouldn't want any changes to behavior in this kinds of clean-up patches.

Comment on attachment 540508 [details] [diff] [review] Remove from nsXULPopupManager >+nsXULPopupManager::HandleEvent(nsIDOMEvent* aEvent) >+{ nsIDOMKeyEvent check somewhere here and pass the keyevent to the KeyUp/Down/Press methods? >+ nsAutoString eventType; >+ aEvent->GetType(eventType); >+ if (eventType.EqualsLiteral("keyup")) >+ return KeyUp(aEvent); >+ if (eventType.EqualsLiteral("keydown")) >+ return KeyDown(aEvent); >+ if (eventType.EqualsLiteral("keypress")) >+ return KeyPress(aEvent); if (expr) { stmt; }

Comment on attachment 540519 [details] [diff] [review] Remove from nsFileControlFrame >diff --git a/layout/forms/nsFileControlFrame.cpp b/layout/forms/nsFileControlFrame.cpp >--- a/layout/forms/nsFileControlFrame.cpp >+++ b/layout/forms/nsFileControlFrame.cpp >@@ -49,17 +49,16 @@ > #include "nsHTMLParts.h" > #include "nsIDOMHTMLInputElement.h" > #include "nsIFormControl.h" > #include "nsINameSpaceManager.h" > #include "nsCOMPtr.h" > #include "nsIDOMElement.h" > #include "nsIDOMDocument.h" > #include "nsIDocument.h" >-#include "nsIDOMMouseListener.h" > #include "nsIPresShell.h" > #include "nsXPCOM.h" > #include "nsISupportsPrimitives.h" > #include "nsIComponentManager.h" > #include "nsPIDOMWindow.h" > #include "nsIFilePicker.h" > #include "nsIDOMMouseEvent.h" > #include "nsINodeInfo.h" >@@ -386,17 +385,17 @@ nsFileControlFrame::SetFocus(PRBool aOn, > { > } > > PRBool ShouldProcessMouseClick(nsIDOMEvent* aMouseEvent) > { > // only allow the left button > nsCOMPtr<nsIDOMMouseEvent> mouseEvent = do_QueryInterface(aMouseEvent); > nsCOMPtr<nsIDOMNSUIEvent> uiEvent = do_QueryInterface(aMouseEvent); >- NS_ENSURE_STATE(uiEvent); >+ NS_ENSURE_TRUE(mouseEvent && uiEvent, PR_FALSE); > PRBool defaultPrevented = PR_FALSE; > uiEvent->GetPreventDefault(&defaultPrevented); > if (defaultPrevented) { > return PR_FALSE; > } > > PRUint16 whichButton; > if (NS_FAILED(mouseEvent->GetButton(&whichButton)) || whichButton != 0) { >@@ -410,17 +409,17 @@ PRBool ShouldProcessMouseClick(nsIDOMEve > > return PR_TRUE; > } > > /** > * This is called when our capture button is clicked > */ > NS_IMETHODIMP >-nsFileControlFrame::CaptureMouseListener::MouseClick(nsIDOMEvent* aMouseEvent) >+nsFileControlFrame::CaptureMouseListener::HandleEvent(nsIDOMEvent* aMouseEvent) > { > nsresult rv; > > NS_ASSERTION(mFrame, "We should have been unregistered"); > if (!ShouldProcessMouseClick(aMouseEvent)) > return NS_OK; > > // Get parent nsIDOMWindowInternal object. >@@ -498,54 +497,48 @@ nsFileControlFrame::CaptureMouseListener > // May need to fire an onchange here > mFrame->mTextFrame->CheckFireOnChange(); > } > > return NS_OK; > } > > /** >- * This is called when our browse button is clicked >- */ >-NS_IMETHODIMP >-nsFileControlFrame::BrowseMouseListener::MouseClick(nsIDOMEvent* aMouseEvent) >-{ >- NS_ASSERTION(mFrame, "We should have been unregistered"); >- if (!ShouldProcessMouseClick(aMouseEvent)) >- return NS_OK; >- >- nsHTMLInputElement* input = >- nsHTMLInputElement::FromContent(mFrame->GetContent()); >- return input ? input->FireAsyncClickHandler() : NS_OK; >-} >- >-/** > * This is called when we receive any registered events on the control. >- * We've only registered for drop, dragover and click events, and click events >- * already call MouseClick() for us. Here, we handle file drops. >+ * We've only registered for drop, dragover and click events. > */ > NS_IMETHODIMP > nsFileControlFrame::BrowseMouseListener::HandleEvent(nsIDOMEvent* aEvent) > { > NS_ASSERTION(mFrame, "We should have been unregistered"); >+ >+ nsAutoString eventType; >+ aEvent->GetType(eventType); >+ if (eventType.EqualsLiteral("click")) { >+ if (!ShouldProcessMouseClick(aEvent)) >+ return NS_OK; >+ >+ nsHTMLInputElement* input = >+ nsHTMLInputElement::FromContent(mFrame->GetContent()); >+ return input ? input->FireAsyncClickHandler() : NS_OK; >+ } >+ > nsCOMPtr<nsIDOMNSUIEvent> uiEvent = do_QueryInterface(aEvent); > NS_ENSURE_STATE(uiEvent); > PRBool defaultPrevented = PR_FALSE; > uiEvent->GetPreventDefault(&defaultPrevented); > if (defaultPrevented) { > return NS_OK; > } > > nsCOMPtr<nsIDOMDragEvent> dragEvent = do_QueryInterface(aEvent); > if (!dragEvent || !IsValidDropData(dragEvent)) { > return NS_OK; > } > >- nsAutoString eventType; >- aEvent->GetType(eventType); > if (eventType.EqualsLiteral("dragover")) { > // Prevent default if we can accept this drag data > aEvent->PreventDefault(); > return NS_OK; > } > > if (eventType.EqualsLiteral("drop")) { > aEvent->StopPropagation(); >@@ -833,11 +826,10 @@ nsFileControlFrame::ParseAcceptAttribute > // Empty loop body because aCallback is doing the work > while (tokenizer.hasMoreTokens() && > (*aCallback)(tokenizer.nextToken(), aClosure)); > } > > //////////////////////////////////////////////////////////// > // Mouse listener implementation > >-NS_IMPL_ISUPPORTS2(nsFileControlFrame::MouseListener, >- nsIDOMMouseListener, >+NS_IMPL_ISUPPORTS1(nsFileControlFrame::MouseListener, > nsIDOMEventListener) >diff --git a/layout/forms/nsFileControlFrame.h b/layout/forms/nsFileControlFrame.h >--- a/layout/forms/nsFileControlFrame.h >+++ b/layout/forms/nsFileControlFrame.h >@@ -35,17 +35,17 @@ > * > * ***** END LICENSE BLOCK ***** */ > > #ifndef nsFileControlFrame_h___ > #define nsFileControlFrame_h___ > > #include "nsBlockFrame.h" > #include "nsIFormControlFrame.h" >-#include "nsIDOMMouseListener.h" >+#include "nsIDOMEventListener.h" > #include "nsIAnonymousContentCreator.h" > #include "nsICapturePicker.h" > #include "nsCOMPtr.h" > > #include "nsTextControlFrame.h" > typedef nsTextControlFrame nsNewFrame; > > class nsIDOMDragEvent; >@@ -107,37 +107,27 @@ public: > void ParseAcceptAttribute(AcceptAttrCallback aCallback, void* aClosure) const; > > nsIFrame* GetTextFrame() { return mTextFrame; } > > protected: > > class MouseListener; > friend class MouseListener; >- class MouseListener : public nsIDOMMouseListener { >+ class MouseListener : public nsIDOMEventListener { > public: > NS_DECL_ISUPPORTS > > MouseListener(nsFileControlFrame* aFrame) : > mFrame(aFrame) > {} > > void ForgetFrame() { > mFrame = nsnull; > } >- >- // We just want to capture the click events on our browse button >- // and textfield. >- NS_IMETHOD MouseDown(nsIDOMEvent* aMouseEvent) { return NS_OK; } >- NS_IMETHOD MouseUp(nsIDOMEvent* aMouseEvent) { return NS_OK; } >- NS_IMETHOD MouseClick(nsIDOMEvent* aMouseEvent) = 0; >- NS_IMETHOD MouseDblClick(nsIDOMEvent* aMouseEvent) { return NS_OK; } >- NS_IMETHOD MouseOver(nsIDOMEvent* aMouseEvent) { return NS_OK; } >- NS_IMETHOD MouseOut(nsIDOMEvent* aMouseEvent) { return NS_OK; } >- NS_IMETHOD HandleEvent(nsIDOMEvent* aEvent) { return NS_OK; } > > protected: > nsFileControlFrame* mFrame; > }; > > class SyncDisabledStateEvent; > friend class SyncDisabledStateEvent; > class SyncDisabledStateEvent : public nsRunnable >@@ -158,27 +148,26 @@ protected: > private: > nsWeakFrame mFrame; > }; > > class CaptureMouseListener: public MouseListener { > public: > CaptureMouseListener(nsFileControlFrame* aFrame) : MouseListener(aFrame), > mMode(0) {}; >- NS_IMETHOD MouseClick(nsIDOMEvent* aMouseEvent); >+ NS_DECL_NSIDOMEVENTLISTENER > PRUint32 mMode; > }; > > class BrowseMouseListener: public MouseListener { > public: > BrowseMouseListener(nsFileControlFrame* aFrame) : MouseListener(aFrame) {}; >- NS_IMETHOD MouseClick(nsIDOMEvent* aMouseEvent); >- NS_IMETHOD HandleEvent(nsIDOMEvent* aEvent); >+ NS_DECL_NSIDOMEVENTLISTENER > >- static PRBool IsValidDropData(nsIDOMDragEvent* aEvent); >+ static PRBool IsValidDropData(nsIDOMDragEvent* aEvent); > }; > > virtual PRBool IsFrameOfType(PRUint32 aFlags) const > { > return nsBlockFrame::IsFrameOfType(aFlags & > ~(nsIFrame::eReplaced | nsIFrame::eReplacedContainsBlock)); > } >

Comment on attachment 540531 [details] [diff] [review] Remove from nsMenuBarListener >+ >+ NS_ABORT(); >+ > return NS_OK; Although it doesn't matter, this sure looks strange NS_ABORT(); return NS_OK; But that is still ok.

(In reply to comment #14) > Comment on attachment 539095 [details] [diff] [review] [review] > Remove from mathml code > > > >+nsMathMLmactionFrame::MouseListener::HandleEvent(nsIDOMEvent* aEvent) > > { > >- mOwner->MouseOver(); > Could you add nsCOMPtr<nsIDOMMouseEvent> check here? Again, is this really needed given that we don't use the event object? Yes, I know that it means that we'll change behavior if there's buggy chrome code that dispatches mouse events using the wrong interface, but that doesn't seem important enough to protect against. > >+ nsAutoString eventType; > >+ aEvent->GetType(eventType); > >+ if (eventType.EqualsLiteral("mouseover")) { > >+ mOwner->MouseOver(); > >+ } > >+ else if (eventType.EqualsLiteral("mouseclick")) { > >+ mOwner->MouseClick(); > >+ } > >+ else if (eventType.EqualsLiteral("mouseout")) { > >+ mOwner->MouseOut(); > >+ } > >+ else { > >+ NS_ABORT(); > >+ } > > if (expr) { > stmt; > } else if (expr) { > stmt; > } That doesn't match the style of the rest of this file (with one exception)

Checked in the first 4 patches: http://hg.mozilla.org/mozilla-central/rev/49d9e0f08eb5 http://hg.mozilla.org/mozilla-central/rev/2bf9296c7bd0 http://hg.mozilla.org/mozilla-central/rev/a41df1f97f0d http://hg.mozilla.org/mozilla-central/rev/8ed7833d498b

Checked in 4 more: http://hg.mozilla.org/mozilla-central/rev/adb41b3796e6 http://hg.mozilla.org/mozilla-central/rev/2c71a7c1cb48 http://hg.mozilla.org/mozilla-central/rev/a45f99f995c2 http://hg.mozilla.org/mozilla-central/rev/6f811ab91ac2

Checked in the last two to mozilla-inbound! http://hg.mozilla.org/integration/mozilla-inbound/rev/e3cb39d085f3 http://hg.mozilla.org/integration/mozilla-inbound/rev/4e08121dcd1e Thanks for all the reviews!!!

http://hg.mozilla.org/mozilla-central/rev/e3cb39d085f3 http://hg.mozilla.org/mozilla-central/rev/4e08121dcd1e

After landing this patches was this bug fixed or it has a major dependency on 678708? Thanks.