From 225a24f0bce34e54cc8289fa77de87ada99bf441 Mon Sep 17 00:00:00 2001 From: Bogdan Tara Date: Wed, 2 Dec 2020 21:16:28 +0200 Subject: [PATCH] Backed out 3 changesets (bug 1677263, bug 1679460, bug 1679461) for test_focus.xhtml failures CLOSED TREE Backed out changeset 03db12dabc63 (bug 1677263) Backed out changeset 75de13448090 (bug 1679460) Backed out changeset 4f4fd8e7ce93 (bug 1679461) --- dom/html/HTMLInputElement.cpp | 49 +++++++++++-------- dom/html/HTMLInputElement.h | 2 +- dom/html/HTMLTextAreaElement.cpp | 35 ++++++++++--- dom/html/HTMLTextAreaElement.h | 2 +- dom/html/TextControlState.cpp | 5 ++ dom/tests/mochitest/chrome/window_focus.xhtml | 9 ++++ .../textfieldselection/select-event.html | 26 +--------- .../forms/textfieldselection/selection.html | 20 ++++++-- 8 files changed, 91 insertions(+), 57 deletions(-) diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 69934dc84830..111f99176783 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -3020,11 +3020,17 @@ void HTMLInputElement::Select() { return; } - TextControlState* state = GetEditorState(); - MOZ_ASSERT(state, "Single line text controls are expected to have a state"); + // XXX Bug? We have to give the input focus before contents can be + // selected - if (FocusState() != eUnfocusable) { - RefPtr fs = state->GetConstFrameSelection(); + FocusTristate state = FocusState(); + if (state == eUnfocusable) { + return; + } + + TextControlState* tes = GetEditorState(); + if (tes) { + RefPtr fs = tes->GetConstFrameSelection(); if (fs && fs->MouseDownRecorded()) { // This means that we're being called while the frame selection has a // mouse down event recorded to adjust the caret during the mouse up @@ -3033,24 +3039,27 @@ void HTMLInputElement::Select() { // select() call takes effect. fs->SetDelayedCaretData(nullptr); } - - if (RefPtr fm = nsFocusManager::GetFocusManager()) { - fm->SetFocus(this, nsIFocusManager::FLAG_NOSCROLL); - - // A focus event handler may change the type attribute, which will destroy - // the previous state object. - state = GetEditorState(); - if (!state) { - return; - } - } } - // Directly call TextControlState::SetSelectionRange because - // HTMLInputElement::SetSelectionRange only applies to fewer types - state->SetSelectionRange(0, UINT32_MAX, - nsITextControlFrame::SelectionDirection::eNone, - IgnoredErrorResult()); + nsFocusManager* fm = nsFocusManager::GetFocusManager(); + + RefPtr presContext = GetPresContext(eForComposedDoc); + if (state == eInactiveWindow) { + if (fm) fm->SetFocus(this, nsIFocusManager::FLAG_NOSCROLL); + SelectAll(presContext); + return; + } + + DispatchSelectEvent(presContext); + if (fm) { + fm->SetFocus(this, nsIFocusManager::FLAG_NOSCROLL); + + // ensure that the element is actually focused + if (this == fm->GetFocusedElement()) { + // Now Select all the text! + SelectAll(presContext); + } + } } void HTMLInputElement::DispatchSelectEvent(nsPresContext* aPresContext) { diff --git a/dom/html/HTMLInputElement.h b/dom/html/HTMLInputElement.h index ed817a182eba..b9d6cccb2203 100644 --- a/dom/html/HTMLInputElement.h +++ b/dom/html/HTMLInputElement.h @@ -670,7 +670,7 @@ class HTMLInputElement final : public TextControlElement, already_AddRefed GetLabels(); - MOZ_CAN_RUN_SCRIPT void Select(); + void Select(); Nullable GetSelectionStart(ErrorResult& aRv); MOZ_CAN_RUN_SCRIPT void SetSelectionStart(const Nullable& aValue, diff --git a/dom/html/HTMLTextAreaElement.cpp b/dom/html/HTMLTextAreaElement.cpp index 538d3f761d04..5a7c9da20f95 100644 --- a/dom/html/HTMLTextAreaElement.cpp +++ b/dom/html/HTMLTextAreaElement.cpp @@ -132,14 +132,37 @@ nsresult HTMLTextAreaElement::Clone(dom::NodeInfo* aNodeInfo, // nsIContent void HTMLTextAreaElement::Select() { - if (FocusState() != eUnfocusable) { - if (RefPtr fm = nsFocusManager::GetFocusManager()) { - fm->SetFocus(this, nsIFocusManager::FLAG_NOSCROLL); - } + // XXX Bug? We have to give the input focus before contents can be + // selected + + FocusTristate state = FocusState(); + if (state == eUnfocusable) { + return; } - SetSelectionRange(0, UINT32_MAX, mozilla::dom::Optional(), - IgnoredErrorResult()); + nsFocusManager* fm = nsFocusManager::GetFocusManager(); + + RefPtr presContext = GetPresContext(eForComposedDoc); + if (state == eInactiveWindow) { + if (fm) fm->SetFocus(this, nsIFocusManager::FLAG_NOSCROLL); + SelectAll(presContext); + return; + } + + WidgetGUIEvent event(true, eFormSelect, nullptr); + // XXXbz HTMLInputElement guards against this reentering; shouldn't we? + EventDispatcher::Dispatch(static_cast(this), presContext, + &event); + + if (fm) { + fm->SetFocus(this, nsIFocusManager::FLAG_NOSCROLL); + + // ensure that the element is actually focused + if (this == fm->GetFocusedElement()) { + // Now Select all the text! + SelectAll(presContext); + } + } } NS_IMETHODIMP diff --git a/dom/html/HTMLTextAreaElement.h b/dom/html/HTMLTextAreaElement.h index 4f92f80ea823..0bdb03cf58bf 100644 --- a/dom/html/HTMLTextAreaElement.h +++ b/dom/html/HTMLTextAreaElement.h @@ -254,7 +254,7 @@ class HTMLTextAreaElement final : public TextControlElement, // via bindings. void SetCustomValidity(const nsAString& aError); - MOZ_CAN_RUN_SCRIPT void Select(); + void Select(); Nullable GetSelectionStart(ErrorResult& aError); MOZ_CAN_RUN_SCRIPT void SetSelectionStart( const Nullable& aSelectionStart, ErrorResult& aError); diff --git a/dom/html/TextControlState.cpp b/dom/html/TextControlState.cpp index e178595b4500..031e81ce2306 100644 --- a/dom/html/TextControlState.cpp +++ b/dom/html/TextControlState.cpp @@ -2212,6 +2212,11 @@ void TextControlState::SetSelectionDirection(const nsAString& aDirection, nsITextControlFrame::SelectionDirection dir = DirectionStringToSelectionDirection(aDirection); + if (IsSelectionCached()) { + GetSelectionProperties().SetDirection(dir); + return; + } + uint32_t start, end; GetSelectionRange(&start, &end, aRv); if (aRv.Failed()) { diff --git a/dom/tests/mochitest/chrome/window_focus.xhtml b/dom/tests/mochitest/chrome/window_focus.xhtml index 6661ac2ee084..a07d519af862 100644 --- a/dom/tests/mochitest/chrome/window_focus.xhtml +++ b/dom/tests/mochitest/chrome/window_focus.xhtml @@ -426,6 +426,15 @@ function startTest() is(t19.selectionEnd, 5, "input focused from tab key selectionEnd"); t19.setSelectionRange(0, 0); + gLastFocusMethod = 0; + var selectFired = false; + function selectListener() { selectFired = true; } + t19.addEventListener("select", selectListener, false); + expectFocusShift(() => t19.select(), + null, getById("t" + 19), true, "input.select()"); + t19.removeEventListener("select", selectListener, false); + ok(selectFired, "select event fires for input"); + // mouse clicking gLastFocusMethod = fm.FLAG_BYMOUSE; for (idx = kTabbableSteps; idx >= 1; idx--) { diff --git a/testing/web-platform/tests/html/semantics/forms/textfieldselection/select-event.html b/testing/web-platform/tests/html/semantics/forms/textfieldselection/select-event.html index 79be87a56a20..2fb018f82aab 100644 --- a/testing/web-platform/tests/html/semantics/forms/textfieldselection/select-event.html +++ b/testing/web-platform/tests/html/semantics/forms/textfieldselection/select-event.html @@ -43,7 +43,7 @@ const actions = [ }, { label: "setRangeText()", - action: el => el.setRangeText("newmiddle", el.selectionStart, el.selectionEnd, "select") + action: el => el.setRangeText("newmiddle") } ]; @@ -79,30 +79,6 @@ els.forEach((el) => { }, 200); }); }, `${elLabel}: ${action.label} a second time (must not fire select)`); - - promise_test(async t => { - const element = el.cloneNode(true); - - element.onselect = e => { - element.onselect = null; - }; - - action.action(element); - - // step_wait properly timeouts before the whole test collapses - await t.step_wait(() => !element.onselect, "event didn't fire", 200, 10); - }, `${elLabel}: ${action.label} disconnected node`); - - // Intentionally still using promise_test, as assert_unreachable does not - // make the test fail inside a listener while t.unreached_func() does. - promise_test(async t => { - const element = el.cloneNode(true); - const listener = t.unreached_func("the select event must not fire synchronously"); - - element.onselect = listener; - action.action(element); - element.onselect = null; - }, `${elLabel}: ${action.label} event queue`); }); }); diff --git a/testing/web-platform/tests/html/semantics/forms/textfieldselection/selection.html b/testing/web-platform/tests/html/semantics/forms/textfieldselection/selection.html index e216d953cef5..b5cea0511d12 100644 --- a/testing/web-platform/tests/html/semantics/forms/textfieldselection/selection.html +++ b/testing/web-platform/tests/html/semantics/forms/textfieldselection/selection.html @@ -95,9 +95,12 @@ // to the character that immediately follows the text entry cursor. assert_equals(el.selectionStart, el.value.length, "SelectionStart offset without selection in " + el.id); + if (!el.parentNode) { + return; + } el.select(); assert_equals(el.selectionStart, 0, "SelectionStart offset"); - el.remove(); + el.parentNode.removeChild(el); }, "test SelectionStart offset for input that is " + (append ? "appended" : " not appended")); } @@ -109,9 +112,12 @@ // to the character that immediately follows the text entry cursor. assert_equals(el.selectionStart, el.value.length, "SelectionStart offset without selection in " + el.id); + if (!el.parentNode) { + return; + } el.select(); assert_equals(el.selectionStart, 0, "SelectionStart offset"); - el.remove(); + el.parentNode.removeChild(el); }, "test SelectionStart offset for textarea that is " + (append ? "appended" : " not appended")); } @@ -123,9 +129,12 @@ // to the character that immediately follows the text entry cursor. assert_equals(el.selectionEnd, el.value.length, "SelectionEnd offset without selection in " + el.id); + if (!el.parentNode) { + return; + } el.select(); assert_equals(el.selectionEnd, el.value.length, "SelectionEnd offset"); - el.remove(); + el.parentNode.removeChild(el); }, "test SelectionEnd offset for input that is " + (append ? "appended" : " not appended")); } @@ -138,9 +147,12 @@ // to the character that immediately follows the text entry cursor. assert_equals(el.selectionEnd, el.value.length, "SelectionEnd offset without selection in " + el.id); + if (!el.parentNode) { + return; + } el.select(); assert_equals(el.selectionEnd, el.value.length, "SelectionEnd offset"); - el.remove(); + el.parentNode.removeChild(el); }, "test SelectionEnd offset for textarea that is " + (append ? "appended" : " not appended")); }