From 032b6f7c896b156f5dab08409dee38ed114ba424 Mon Sep 17 00:00:00 2001 From: Ciure Andrei Date: Sun, 15 Mar 2020 16:04:36 +0200 Subject: [PATCH] Backed out 2 changesets (bug 1620778) for causing test_autocomplete_mac_caret.xhtml failures Backed out changeset 66f97d1cf94a (bug 1620778) Backed out changeset eda75d901f4c (bug 1620778) --- dom/html/HTMLInputElement.cpp | 59 ++++----- dom/html/HTMLInputElement.h | 4 - dom/html/TextControlState.cpp | 9 -- editor/libeditor/tests/mochitest.ini | 1 - editor/libeditor/tests/test_bug1620778.html | 27 ----- .../autocomplete/nsAutoCompleteController.cpp | 113 ++++++++---------- .../chrome/test_autocomplete_mac_caret.xhtml | 4 +- 7 files changed, 77 insertions(+), 140 deletions(-) delete mode 100644 editor/libeditor/tests/test_bug1620778.html diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 50819fe553f0..53e7b11609c6 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -3579,35 +3579,10 @@ nsresult HTMLInputElement::MaybeInitPickers(EventChainPostVisitor& aVisitor) { * Control is treated specially, since sometimes we ignore it, and sometimes * we don't (for webcompat reasons). */ -static bool IgnoreInputEventWithModifier(const WidgetInputEvent& aEvent, +static bool IgnoreInputEventWithModifier(WidgetInputEvent* aEvent, bool ignoreControl) { - return (ignoreControl && aEvent.IsControl()) || aEvent.IsAltGraph() || - aEvent.IsFn() || aEvent.IsOS(); -} - -bool HTMLInputElement::StepsInputValue(const WidgetKeyboardEvent& aEvent) const { - if (mType != NS_FORM_INPUT_NUMBER) { - return false; - } - if (aEvent.mMessage != eKeyPress) { - return false; - } - if (!aEvent.IsTrusted()) { - return false; - } - if (aEvent.mKeyCode != NS_VK_UP && aEvent.mKeyCode != NS_VK_DOWN) { - return false; - } - if (IgnoreInputEventWithModifier(aEvent, false)) { - return false; - } - if (aEvent.DefaultPrevented()) { - return false; - } - if (!IsMutable()) { - return false; - } - return true; + return (ignoreControl && aEvent->IsControl()) || aEvent->IsAltGraph() || + aEvent->IsFn() || aEvent->IsOS(); } nsresult HTMLInputElement::PostHandleEvent(EventChainPostVisitor& aVisitor) { @@ -3734,10 +3709,26 @@ nsresult HTMLInputElement::PostHandleEvent(EventChainPostVisitor& aVisitor) { if (NS_SUCCEEDED(rv)) { WidgetKeyboardEvent* keyEvent = aVisitor.mEvent->AsKeyboardEvent(); - if (keyEvent && StepsInputValue(*keyEvent)) { - StepNumberControlForUserEvent(keyEvent->mKeyCode == NS_VK_UP ? 1 : -1); - FireChangeEventIfNeeded(); - aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault; + if (mType == NS_FORM_INPUT_NUMBER && keyEvent && + keyEvent->mMessage == eKeyPress && aVisitor.mEvent->IsTrusted() && + (keyEvent->mKeyCode == NS_VK_UP || keyEvent->mKeyCode == NS_VK_DOWN) && + !IgnoreInputEventWithModifier(keyEvent, false)) { + // We handle the up/down arrow keys specially for . + // On some platforms the editor for the nested text control will + // process these keys to send the cursor to the start/end of the text + // control and as a result aVisitor.mEventStatus will already have been + // set to nsEventStatus_eConsumeNoDefault. However, we know that + // whenever the up/down arrow keys cause the value of the number + // control to change the string in the text control will change, and + // the cursor will be moved to the end of the text control, overwriting + // the editor's handling of up/down keypress events. For that reason we + // just ignore aVisitor.mEventStatus here and go ahead and handle the + // event to increase/decrease the value of the number control. + if (!aVisitor.mEvent->DefaultPreventedByContent() && IsMutable()) { + StepNumberControlForUserEvent(keyEvent->mKeyCode == NS_VK_UP ? 1 : -1); + FireChangeEventIfNeeded(); + aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault; + } } else if (nsEventStatus_eIgnore == aVisitor.mEventStatus) { switch (aVisitor.mEvent->mMessage) { case eFocus: { @@ -3950,7 +3941,7 @@ nsresult HTMLInputElement::PostHandleEvent(EventChainPostVisitor& aVisitor) { } if (mType == NS_FORM_INPUT_NUMBER && aVisitor.mEvent->IsTrusted()) { if (mouseEvent->mButton == MouseButton::eLeft && - !IgnoreInputEventWithModifier(*mouseEvent, false)) { + !IgnoreInputEventWithModifier(mouseEvent, false)) { nsNumberControlFrame* numberControlFrame = do_QueryFrame(GetPrimaryFrame()); if (numberControlFrame) { @@ -4097,7 +4088,7 @@ void HTMLInputElement::PostHandleEventForRangeThumb( break; // don't start drag if someone else is already capturing } WidgetInputEvent* inputEvent = aVisitor.mEvent->AsInputEvent(); - if (IgnoreInputEventWithModifier(*inputEvent, true)) { + if (IgnoreInputEventWithModifier(inputEvent, true)) { break; // ignore } if (aVisitor.mEvent->mMessage == eMouseDown) { diff --git a/dom/html/HTMLInputElement.h b/dom/html/HTMLInputElement.h index 5f39932d26cf..1f22643a1da7 100644 --- a/dom/html/HTMLInputElement.h +++ b/dom/html/HTMLInputElement.h @@ -667,10 +667,6 @@ class HTMLInputElement final : public TextControlElement, */ Decimal GetStep() const; - // Returns whether the given keyboard event steps up or down the value of an - // element. - bool StepsInputValue(const WidgetKeyboardEvent&) const; - already_AddRefed GetLabels(); void Select(); diff --git a/dom/html/TextControlState.cpp b/dom/html/TextControlState.cpp index 5a8328de449f..ea478ea7169b 100644 --- a/dom/html/TextControlState.cpp +++ b/dom/html/TextControlState.cpp @@ -949,15 +949,6 @@ TextInputListener::HandleEvent(Event* aEvent) { return NS_ERROR_UNEXPECTED; } - { - auto* input = HTMLInputElement::FromNode(mTxtCtrlElement); - if (input && input->StepsInputValue(*widgetKeyEvent)) { - // As an special case, don't handle key events that would step the value - // of our . - return NS_OK; - } - } - KeyEventHandler* keyHandlers = ShortcutKeys::GetHandlers( mTxtCtrlElement->IsTextArea() ? HandlerType::eTextArea : HandlerType::eInput); diff --git a/editor/libeditor/tests/mochitest.ini b/editor/libeditor/tests/mochitest.ini index baceaf850e39..d878d54c7b76 100644 --- a/editor/libeditor/tests/mochitest.ini +++ b/editor/libeditor/tests/mochitest.ini @@ -227,7 +227,6 @@ skip-if = toolkit == 'android' skip-if = os == "android" #Bug 1575739 [test_bug1581337.html] [test_bug1619852.html] -[test_bug1620778.html] [test_abs_positioner_appearance.html] [test_abs_positioner_positioning_elements.html] skip-if = os == 'android' # Bug 1525959 diff --git a/editor/libeditor/tests/test_bug1620778.html b/editor/libeditor/tests/test_bug1620778.html deleted file mode 100644 index 5a1c34fb1a64..000000000000 --- a/editor/libeditor/tests/test_bug1620778.html +++ /dev/null @@ -1,27 +0,0 @@ - -Test for Bug 1620778 - - - - - - - - diff --git a/toolkit/components/autocomplete/nsAutoCompleteController.cpp b/toolkit/components/autocomplete/nsAutoCompleteController.cpp index bb326127077a..b70ea0113a19 100644 --- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp +++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp @@ -420,12 +420,13 @@ nsAutoCompleteController::HandleKeyNavigation(uint32_t aKey, bool* _retval) { aKey == dom::KeyboardEvent_Binding::DOM_VK_DOWN || aKey == dom::KeyboardEvent_Binding::DOM_VK_PAGE_UP || aKey == dom::KeyboardEvent_Binding::DOM_VK_PAGE_DOWN) { + // Prevent the input from handling up/down events, as it may move + // the cursor to home/end on some systems + *_retval = true; + bool isOpen = false; input->GetPopupOpen(&isOpen); if (isOpen) { - // Prevent the input from handling up/down events, as it may move - // the cursor to home/end on some systems - *_retval = true; bool reverse = aKey == dom::KeyboardEvent_Binding::DOM_VK_UP || aKey == dom::KeyboardEvent_Binding::DOM_VK_PAGE_UP ? true @@ -487,72 +488,60 @@ nsAutoCompleteController::HandleKeyNavigation(uint32_t aKey, bool* _retval) { } } } else { - // Only show the popup if the caret is at the start or end of the input - // and there is no selection, so that the default defined key shortcuts - // for up and down move to the beginning and end of the field otherwise. - if (aKey == dom::KeyboardEvent_Binding::DOM_VK_UP || - aKey == dom::KeyboardEvent_Binding::DOM_VK_DOWN) { - const bool isUp = aKey == dom::KeyboardEvent_Binding::DOM_VK_UP; - - int32_t start, end; +#ifdef XP_MACOSX + // on Mac, only show the popup if the caret is at the start or end of + // the input and there is no selection, so that the default defined key + // shortcuts for up and down move to the beginning and end of the field + // otherwise. + int32_t start, end; + if (aKey == dom::KeyboardEvent_Binding::DOM_VK_UP) { input->GetSelectionStart(&start); input->GetSelectionEnd(&end); + if (start > 0 || start != end) *_retval = false; + } else if (aKey == dom::KeyboardEvent_Binding::DOM_VK_DOWN) { + nsAutoString text; + input->GetTextValue(text); + input->GetSelectionStart(&start); + input->GetSelectionEnd(&end); + if (start != end || end < (int32_t)text.Length()) *_retval = false; + } +#endif + if (*_retval) { + nsAutoString oldSearchString; + uint16_t oldResult = 0; - if (isUp) { - if (start > 0 || start != end) { - return NS_OK; + // Open the popup if there has been a previous non-errored search, or + // else kick off a new search + if (!mResults.IsEmpty() && + NS_SUCCEEDED(mResults[0]->GetSearchResult(&oldResult)) && + oldResult != nsIAutoCompleteResult::RESULT_FAILURE && + NS_SUCCEEDED(mResults[0]->GetSearchString(oldSearchString)) && + oldSearchString.Equals(mSearchString, + nsCaseInsensitiveStringComparator())) { + if (mMatchCount) { + OpenPopup(); } } else { - nsAutoString text; - input->GetTextValue(text); - if (start != end || end < (int32_t)text.Length()) { + // Stop all searches in case they are async. + StopSearch(); + + if (!mInput) { + // StopSearch() can call PostSearchCleanup() which might result + // in a blur event, which could null out mInput, so we need to check + // it again. See bug #395344 for more details return NS_OK; } + + // Some script may have changed the value of the text field since our + // last keypress or after our focus handler and we don't want to + // search for a stale string. + nsAutoString value; + input->GetTextValue(value); + SetSearchStringInternal(value); + + StartSearches(); } } - - nsAutoString oldSearchString; - uint16_t oldResult = 0; - - // Open the popup if there has been a previous non-errored search, or - // else kick off a new search - if (!mResults.IsEmpty() && - NS_SUCCEEDED(mResults[0]->GetSearchResult(&oldResult)) && - oldResult != nsIAutoCompleteResult::RESULT_FAILURE && - NS_SUCCEEDED(mResults[0]->GetSearchString(oldSearchString)) && - oldSearchString.Equals(mSearchString, - nsCaseInsensitiveStringComparator())) { - if (mMatchCount) { - OpenPopup(); - } - } else { - // Stop all searches in case they are async. - StopSearch(); - - if (!mInput) { - // StopSearch() can call PostSearchCleanup() which might result - // in a blur event, which could null out mInput, so we need to check - // it again. See bug #395344 for more details - return NS_OK; - } - - // Some script may have changed the value of the text field since our - // last keypress or after our focus handler and we don't want to - // search for a stale string. - nsAutoString value; - input->GetTextValue(value); - SetSearchStringInternal(value); - - StartSearches(); - } - - bool isOpen = false; - input->GetPopupOpen(&isOpen); - if (isOpen) { - // Prevent the default action if we opened the popup in any of the code - // paths above. - *_retval = true; - } } } else if (aKey == dom::KeyboardEvent_Binding::DOM_VK_LEFT || aKey == dom::KeyboardEvent_Binding::DOM_VK_RIGHT @@ -997,9 +986,7 @@ nsresult nsAutoCompleteController::StartSearch(uint16_t aSearchType) { void nsAutoCompleteController::AfterSearches() { mResultCache.Clear(); - if (mSearchesFailed == mSearches.Length()) { - PostSearchCleanup(); - } + if (mSearchesFailed == mSearches.Length()) PostSearchCleanup(); } NS_IMETHODIMP diff --git a/toolkit/content/tests/chrome/test_autocomplete_mac_caret.xhtml b/toolkit/content/tests/chrome/test_autocomplete_mac_caret.xhtml index 766cbb542509..b44eec0a4e6d 100644 --- a/toolkit/content/tests/chrome/test_autocomplete_mac_caret.xhtml +++ b/toolkit/content/tests/chrome/test_autocomplete_mac_caret.xhtml @@ -22,8 +22,8 @@ function keyCaretTest() var autocomplete = $("autocomplete"); autocomplete.focus(); - checkKeyCaretTest("KEY_ArrowUp", 0, 0, true, "no value up"); - checkKeyCaretTest("KEY_ArrowDown", 0, 0, true, "no value down"); + checkKeyCaretTest("KEY_ArrowUp", 0, 0, false, "no value up"); + checkKeyCaretTest("KEY_ArrowDown", 0, 0, false, "no value down"); autocomplete.value = "Sample";