From 8a8f0df35849ad27cc8ed1ccce8baead46a7656a Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Tue, 26 Jun 2018 14:05:33 +0200 Subject: Issue #12 Part 4: Don't access mEditorBase directly. EditorEventListener should grab mEditorBase in a smaller scope wherever possible and shouldn't access it directly while handling an event. Each event listener method shouldn't access mEditorBase directly when calling its method since it might be changed to another instance. --- editor/libeditor/EditorEventListener.cpp | 93 ++++++++++++++++------------ editor/libeditor/HTMLEditorEventListener.cpp | 2 +- 2 files changed, 54 insertions(+), 41 deletions(-) diff --git a/editor/libeditor/EditorEventListener.cpp b/editor/libeditor/EditorEventListener.cpp index ce7960a05..a6fcfd933 100644 --- a/editor/libeditor/EditorEventListener.cpp +++ b/editor/libeditor/EditorEventListener.cpp @@ -362,9 +362,6 @@ EditorEventListener::DetachedFromEditorOrDefaultPrevented( NS_IMETHODIMP EditorEventListener::HandleEvent(nsIDOMEvent* aEvent) { - nsCOMPtr kungFuDeathGrip = mEditorBase; - Unused << kungFuDeathGrip; // mEditorBase is not referred to in this function - // Let's handle each event with the message of the internal event of the // coming event. If the DOM event was created with improper interface, // e.g., keydown event is created with |new MouseEvent("keydown", {});|, @@ -558,12 +555,13 @@ EditorEventListener::KeyUp(nsIDOMKeyEvent* aKeyEvent) } // XXX Why doesn't this method check if it's consumed? + RefPtr editorBase(mEditorBase); uint32_t keyCode = 0; aKeyEvent->GetKeyCode(&keyCode); if ((keyCode == nsIDOMKeyEvent::DOM_VK_SHIFT || keyCode == nsIDOMKeyEvent::DOM_VK_CONTROL) && - mShouldSwitchTextDirection && mEditorBase->IsPlaintextEditor()) { - mEditorBase->SwitchTextDirectionTo(mSwitchToRTL ? + mShouldSwitchTextDirection && editorBase->IsPlaintextEditor()) { + editorBase->SwitchTextDirectionTo(mSwitchToRTL ? nsIPlaintextEditor::eEditorRightToLeft : nsIPlaintextEditor::eEditorLeftToRight); mShouldSwitchTextDirection = false; @@ -606,16 +604,17 @@ EditorEventListener::KeyPress(nsIDOMKeyEvent* aKeyEvent) return NS_OK; } + RefPtr editorBase(mEditorBase); WidgetKeyboardEvent* keypressEvent = aKeyEvent->AsEvent()->WidgetEventPtr()->AsKeyboardEvent(); MOZ_ASSERT(keypressEvent, "DOM key event's internal event must be WidgetKeyboardEvent"); - if (!mEditorBase->IsAcceptableInputEvent(keypressEvent) || + if (!editorBase->IsAcceptableInputEvent(keypressEvent) || DetachedFromEditorOrDefaultPrevented(keypressEvent)) { return NS_OK; } - nsresult rv = mEditorBase->HandleKeyPressEvent(aKeyEvent); + nsresult rv = editorBase->HandleKeyPressEvent(aKeyEvent); NS_ENSURE_SUCCESS(rv, rv); if (DetachedFromEditorOrDefaultPrevented(keypressEvent)) { return NS_OK; @@ -635,7 +634,7 @@ EditorEventListener::KeyPress(nsIDOMKeyEvent* aKeyEvent) NS_ENSURE_TRUE(widget, NS_OK); } - nsCOMPtr doc = mEditorBase->GetDocument(); + nsCOMPtr doc = editorBase->GetDocument(); bool handled = widget->ExecuteNativeKeyBinding( nsIWidget::NativeKeyBindingsForRichTextEditor, *keypressEvent, DoCommandCallback, doc); @@ -652,10 +651,11 @@ EditorEventListener::MouseClick(nsIDOMMouseEvent* aMouseEvent) return NS_OK; } // nothing to do if editor isn't editable or clicked on out of the editor. + RefPtr editorBase(mEditorBase); WidgetMouseEvent* clickEvent = aMouseEvent->AsEvent()->WidgetEventPtr()->AsMouseEvent(); - if (mEditorBase->IsReadonly() || mEditorBase->IsDisabled() || - !mEditorBase->IsAcceptableInputEvent(clickEvent)) { + if (editorBase->IsReadonly() || editorBase->IsDisabled() || + !editorBase->IsAcceptableInputEvent(clickEvent)) { return NS_OK; } @@ -679,7 +679,7 @@ EditorEventListener::MouseClick(nsIDOMMouseEvent* aMouseEvent) // If we got a mouse down inside the editing area, we should force the // IME to commit before we change the cursor position - mEditorBase->ForceCompositionEnd(); + editorBase->ForceCompositionEnd(); if (DetachedFromEditor()) { return NS_OK; } @@ -714,7 +714,8 @@ EditorEventListener::HandleMiddleClickPaste(nsIDOMMouseEvent* aMouseEvent) return NS_ERROR_NULL_POINTER; } - RefPtr selection = mEditorBase->GetSelection(); + RefPtr editorBase(mEditorBase); + RefPtr selection = editorBase->GetSelection(); if (selection) { selection->Collapse(parent, offset); } @@ -726,7 +727,7 @@ EditorEventListener::HandleMiddleClickPaste(nsIDOMMouseEvent* aMouseEvent) nsCOMPtr mailEditor; if (ctrlKey) { - mailEditor = do_QueryObject(mEditorBase); + mailEditor = do_QueryObject(editorBase); } nsresult rv; @@ -744,7 +745,7 @@ EditorEventListener::HandleMiddleClickPaste(nsIDOMMouseEvent* aMouseEvent) if (mailEditor) { mailEditor->PasteAsQuotation(clipboard); } else { - mEditorBase->Paste(clipboard); + editorBase->Paste(clipboard); } // Prevent the event from propagating up to be possibly handled @@ -783,7 +784,8 @@ EditorEventListener::MouseDown(nsIDOMMouseEvent* aMouseEvent) if (DetachedFromEditor()) { return NS_OK; } - mEditorBase->ForceCompositionEnd(); + RefPtr editorBase(mEditorBase); + editorBase->ForceCompositionEnd(); return NS_OK; } @@ -793,17 +795,18 @@ EditorEventListener::HandleChangeComposition( { MOZ_ASSERT(!aCompositionChangeEvent->DefaultPrevented(), "eCompositionChange event shouldn't be cancelable"); + RefPtr editorBase(mEditorBase); if (DetachedFromEditor() || - !mEditorBase->IsAcceptableInputEvent(aCompositionChangeEvent)) { + !editorBase->IsAcceptableInputEvent(aCompositionChangeEvent)) { return NS_OK; } // if we are readonly or disabled, then do nothing. - if (mEditorBase->IsReadonly() || mEditorBase->IsDisabled()) { + if (editorBase->IsReadonly() || editorBase->IsDisabled()) { return NS_OK; } - return mEditorBase->UpdateIMEComposition(aCompositionChangeEvent); + return editorBase->UpdateIMEComposition(aCompositionChangeEvent); } /** @@ -932,7 +935,8 @@ EditorEventListener::Drop(nsIDOMDragEvent* aDragEvent) if (!dropParent->IsEditable() || !CanDrop(aDragEvent)) { // was it because we're read-only? - if ((mEditorBase->IsReadonly() || mEditorBase->IsDisabled()) && + RefPtr editorBase(mEditorBase); + if ((editorBase->IsReadonly() || editorBase->IsDisabled()) && !IsFileControlTextBox()) { // it was decided to "eat" the event as this is the "least surprise" // since someone else handling it might be unintentional and the @@ -945,7 +949,8 @@ EditorEventListener::Drop(nsIDOMDragEvent* aDragEvent) aDragEvent->AsEvent()->StopPropagation(); aDragEvent->AsEvent()->PreventDefault(); - return mEditorBase->InsertFromDrop(aDragEvent->AsEvent()); + RefPtr editorBase(mEditorBase); + return editorBase->InsertFromDrop(aDragEvent->AsEvent()); } bool @@ -955,7 +960,8 @@ EditorEventListener::CanDrop(nsIDOMDragEvent* aEvent) aEvent->AsEvent()->WidgetEventPtr())); // if the target doc is read-only, we can't drop - if (mEditorBase->IsReadonly() || mEditorBase->IsDisabled()) { + RefPtr editorBase(mEditorBase); + if (editorBase->IsReadonly() || editorBase->IsDisabled()) { return false; } @@ -971,7 +977,7 @@ EditorEventListener::CanDrop(nsIDOMDragEvent* aEvent) // can be dropped as well. if (!types.Contains(NS_LITERAL_STRING(kTextMime)) && !types.Contains(NS_LITERAL_STRING(kMozTextInternal)) && - (mEditorBase->IsPlaintextEditor() || + (editorBase->IsPlaintextEditor() || (!types.Contains(NS_LITERAL_STRING(kHTMLMime)) && !types.Contains(NS_LITERAL_STRING(kFileMime))))) { return false; @@ -989,7 +995,7 @@ EditorEventListener::CanDrop(nsIDOMDragEvent* aEvent) // There is a source node, so compare the source documents and this document. // Disallow drops on the same document. - nsCOMPtr domdoc = mEditorBase->GetDOMDocument(); + nsCOMPtr domdoc = editorBase->GetDOMDocument(); NS_ENSURE_TRUE(domdoc, false); nsCOMPtr sourceDoc; @@ -1009,7 +1015,7 @@ EditorEventListener::CanDrop(nsIDOMDragEvent* aEvent) return true; } - RefPtr selection = mEditorBase->GetSelection(); + RefPtr selection = editorBase->GetSelection(); if (!selection) { return false; } @@ -1054,28 +1060,30 @@ nsresult EditorEventListener::HandleStartComposition( WidgetCompositionEvent* aCompositionStartEvent) { + RefPtr editorBase(mEditorBase); if (DetachedFromEditor() || - !mEditorBase->IsAcceptableInputEvent(aCompositionStartEvent)) { + !editorBase->IsAcceptableInputEvent(aCompositionStartEvent)) { return NS_OK; } // Although, "compositionstart" should be cancelable, but currently, // eCompositionStart event coming from widget is not cancelable. MOZ_ASSERT(!aCompositionStartEvent->DefaultPrevented(), "eCompositionStart shouldn't be cancelable"); - return mEditorBase->BeginIMEComposition(aCompositionStartEvent); + return editorBase->BeginIMEComposition(aCompositionStartEvent); } void EditorEventListener::HandleEndComposition( WidgetCompositionEvent* aCompositionEndEvent) { + RefPtr editorBase(mEditorBase); if (DetachedFromEditor() || - !mEditorBase->IsAcceptableInputEvent(aCompositionEndEvent)) { + !editorBase->IsAcceptableInputEvent(aCompositionEndEvent)) { return; } MOZ_ASSERT(!aCompositionEndEvent->DefaultPrevented(), "eCompositionEnd shouldn't be cancelable"); - mEditorBase->EndIMEComposition(); + editorBase->EndIMEComposition(); } nsresult @@ -1092,13 +1100,14 @@ EditorEventListener::Focus(WidgetEvent* aFocusEvent) "eFocus event shouldn't be cancelable"); // Don't turn on selection and caret when the editor is disabled. - if (mEditorBase->IsDisabled()) { + RefPtr editorBase(mEditorBase); + if (editorBase->IsDisabled()) { return NS_OK; } // Spell check a textarea the first time that it is focused. SpellCheckIfNeeded(); - if (!mEditorBase) { + if (!editorBase) { // In e10s, this can cause us to flush notifications, which can destroy // the node we're about to focus. return NS_OK; @@ -1122,7 +1131,7 @@ EditorEventListener::Focus(WidgetEvent* aFocusEvent) // contenteditable editor. So, the editableRoot value is invalid for // the plain text editor, and it will be set to the wrong limiter of // the selection. However, fortunately, actual bugs are not found yet. - nsCOMPtr editableRoot = mEditorBase->FindSelectionRoot(node); + nsCOMPtr editableRoot = editorBase->FindSelectionRoot(node); // make sure that the element is really focused in case an earlier // listener in the chain changed the focus. @@ -1152,16 +1161,16 @@ EditorEventListener::Focus(WidgetEvent* aFocusEvent) } } - mEditorBase->OnFocus(target); + editorBase->OnFocus(target); if (DetachedFromEditorOrDefaultPrevented(aFocusEvent)) { return NS_OK; } nsCOMPtr ps = GetPresShell(); NS_ENSURE_TRUE(ps, NS_OK); - nsCOMPtr focusedContent = mEditorBase->GetFocusedContentForIME(); + nsCOMPtr focusedContent = editorBase->GetFocusedContentForIME(); IMEStateManager::OnFocusInEditor(ps->GetPresContext(), focusedContent, - mEditorBase); + editorBase); return NS_OK; } @@ -1187,7 +1196,8 @@ EditorEventListener::Blur(WidgetEvent* aBlurEvent) nsCOMPtr element; fm->GetFocusedElement(getter_AddRefs(element)); if (!element) { - mEditorBase->FinalizeSelection(); + RefPtr editorBase(mEditorBase); + editorBase->FinalizeSelection(); } return NS_OK; } @@ -1199,11 +1209,12 @@ EditorEventListener::SpellCheckIfNeeded() // If the spell check skip flag is still enabled from creation time, // disable it because focused editors are allowed to spell check. + RefPtr editorBase(mEditorBase); uint32_t currentFlags = 0; - mEditorBase->GetFlags(¤tFlags); + editorBase->GetFlags(¤tFlags); if(currentFlags & nsIPlaintextEditor::eEditorSkipSpellCheck) { currentFlags ^= nsIPlaintextEditor::eEditorSkipSpellCheck; - mEditorBase->SetFlags(currentFlags); + editorBase->SetFlags(currentFlags); } } @@ -1212,7 +1223,8 @@ EditorEventListener::IsFileControlTextBox() { MOZ_ASSERT(!DetachedFromEditor()); - Element* root = mEditorBase->GetRoot(); + RefPtr editorBase(mEditorBase); + Element* root = editorBase->GetRoot(); if (!root || !root->ChromeOnlyAccess()) { return false; } @@ -1244,13 +1256,14 @@ EditorEventListener::ShouldHandleNativeKeyBindings(nsIDOMKeyEvent* aKeyEvent) return false; } + RefPtr editorBase(mEditorBase); nsCOMPtr htmlEditor = - do_QueryInterface(static_cast(mEditorBase)); + do_QueryInterface(static_cast(editorBase)); if (!htmlEditor) { return false; } - nsCOMPtr doc = mEditorBase->GetDocument(); + nsCOMPtr doc = editorBase->GetDocument(); if (doc->HasFlag(NODE_IS_EDITABLE)) { // Don't need to perform any checks in designMode documents. return true; diff --git a/editor/libeditor/HTMLEditorEventListener.cpp b/editor/libeditor/HTMLEditorEventListener.cpp index a22188483..aa767519c 100644 --- a/editor/libeditor/HTMLEditorEventListener.cpp +++ b/editor/libeditor/HTMLEditorEventListener.cpp @@ -120,7 +120,7 @@ HTMLEditorEventListener::MouseDown(nsIDOMMouseEvent* aMouseEvent) nsCOMPtr element = do_QueryInterface(target); if (isContextClick || (buttonNumber == 0 && clickCount == 2)) { - RefPtr selection = mEditorBase->GetSelection(); + RefPtr selection = htmlEditor->GetSelection(); NS_ENSURE_TRUE(selection, NS_OK); // Get location of mouse within target node -- cgit v1.2.3