Bug 1771806 - Record last interactive value in TextControlState. r=masayuki

One unfortunate-ish thing of storing it in TextControlState is that we
can't preserve it across type changes, but that seems ok for now
according to my conversations with Serg.

In the future we might want to move the value to HTMLInputElement itself
or something, we'll see, but this should help folks fix some
long-standing quality issues in the password manager / autofill
implementation.

Differential Revision: https://phabricator.services.mozilla.com/D150148
This commit is contained in:
Emilio Cobos Álvarez 2022-06-28 08:49:12 +00:00
Родитель d779bb0acd
Коммит 569a522d01
7 изменённых файлов: 204 добавлений и 6 удалений

Просмотреть файл

@ -2733,6 +2733,12 @@ nsresult HTMLInputElement::SetValueInternal(
SetValueChanged(true);
}
// Make sure to keep track of the last value change not being interactive,
// just in case this used to be another kind of editable input before.
// Note that a checked change _could_ really be interactive, but we don't
// keep track of that elsewhere so seems fine to just do this.
SetLastValueChangeWasInteractive(false);
// Treat value == defaultValue for other input elements.
return nsGenericHTMLFormControlElementWithState::SetAttr(
kNameSpaceID_None, nsGkAtoms::value, aValue, true);
@ -5100,6 +5106,17 @@ bool HTMLInputElement::IsDateTimeTypeSupported(
}
}
void HTMLInputElement::GetLastInteractiveValue(nsAString& aValue) {
if (mLastValueChangeWasInteractive) {
return GetValue(aValue, CallerType::System);
}
if (TextControlState* state = GetEditorState()) {
return aValue.Assign(
state->LastInteractiveValueIfLastChangeWasNonInteractive());
}
aValue.Truncate();
}
bool HTMLInputElement::ParseAttribute(int32_t aNamespaceID, nsAtom* aAttribute,
const nsAString& aValue,
nsIPrincipal* aMaybeScriptedPrincipal,

Просмотреть файл

@ -169,6 +169,13 @@ class HTMLInputElement final : public TextControlElement,
const nsAString& aValue,
nsIPrincipal* aMaybeScriptedPrincipal,
nsAttrValue& aResult) override;
bool LastValueChangeWasInteractive() const {
return mLastValueChangeWasInteractive;
}
void GetLastInteractiveValue(nsAString&);
nsChangeHint GetAttributeChangeHint(const nsAtom* aAttribute,
int32_t aModType) const override;
NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;

Просмотреть файл

@ -1083,7 +1083,13 @@ void TextInputListener::HandleValueChanged() {
}
if (!mSettingValue) {
// NOTE(emilio): execCommand might get here even though it might not be a
// "proper" user-interactive change. Might be worth reconsidering which
// ValueChangeKind are we passing down.
mTxtCtrlElement->OnValueChanged(ValueChangeKind::UserInteraction);
if (mTextControlState) {
mTextControlState->ClearLastInteractiveValue();
}
}
}
@ -2643,6 +2649,27 @@ bool TextControlState::SetValue(const nsAString& aValue,
return false;
}
const auto changeKind = [&] {
if (aOptions.contains(ValueSetterOption::ByInternalAPI)) {
return ValueChangeKind::Internal;
}
if (aOptions.contains(ValueSetterOption::BySetUserInputAPI)) {
return ValueChangeKind::UserInteraction;
}
return ValueChangeKind::Script;
}();
if (changeKind == ValueChangeKind::Script) {
// This value change will not be interactive. If we're an input that was
// interactively edited, save the last interactive value now before it goes
// away.
if (auto* input = HTMLInputElement::FromNode(mTextCtrlElement)) {
if (input->LastValueChangeWasInteractive()) {
GetValue(mLastInteractiveValue, /* aIgnoreWrap = */ true);
}
}
}
// Note that if this may be called during reframe of the editor. In such
// case, we shouldn't commit composition. Therefore, when this is called
// for internal processing, we shouldn't commit the composition.
@ -2723,11 +2750,6 @@ bool TextControlState::SetValue(const nsAString& aValue,
// If we were handling SetValue() before, don't update the DOM state twice,
// just let the outer call do so.
if (!wasHandlingSetValue) {
// TODO(emilio): It seems wrong to pass ValueChangeKind::Script if
// BySetUserInput is in aOptions.
auto changeKind = aOptions.contains(ValueSetterOption::ByInternalAPI)
? ValueChangeKind::Internal
: ValueChangeKind::Script;
handlingSetValue.GetTextControlElement()->OnValueChanged(changeKind);
}
return true;
@ -2992,6 +3014,8 @@ bool TextControlState::SetValueWithoutTextEditor(
aHandlingSetValue.GetTextControlElement()->OnValueChanged(
ValueChangeKind::UserInteraction);
ClearLastInteractiveValue();
MOZ_ASSERT(!aHandlingSetValue.GetSettingValue().IsVoid());
DebugOnly<nsresult> rvIgnored = nsContentUtils::DispatchInputEvent(
MOZ_KnownLive(aHandlingSetValue.GetTextControlElement()),

Просмотреть файл

@ -306,6 +306,13 @@ class TextControlState final : public SupportsWeakPtr {
}
bool IsEmpty() const { return mValue.IsEmpty(); }
const nsAString& LastInteractiveValueIfLastChangeWasNonInteractive() const {
return mLastInteractiveValue;
}
// When an interactive value change happens, we clear mLastInteractiveValue
// because it's not needed (mValue is the new interactive value).
void ClearLastInteractiveValue() { mLastInteractiveValue.SetIsVoid(true); }
Element* GetRootNode();
Element* GetPreviewNode();
@ -515,7 +522,12 @@ class TextControlState final : public SupportsWeakPtr {
RefPtr<TextInputListener> mTextListener;
UniquePtr<PasswordMaskData> mPasswordMaskData;
nsString mValue { VoidString() }; // Void if there's no value.
nsString mValue{VoidString()}; // Void if there's no value.
// If our input's last value change was not interactive (as in, the value
// change was caused by a ValueChangeKind::UserInteraction), this is the value
// that the last interaction had.
nsString mLastInteractiveValue{VoidString()};
SelectionProperties mSelectionProperties;

Просмотреть файл

@ -527,6 +527,7 @@ tags = openwindow
[test_ul_attributes_reflection.html]
[test_input_file_cancel_event.html]
[test_input_files_not_nsIFile.html]
[test_input_lastInteractiveValue.html]
[test_fragment_form_pointer.html]
[test_bug1682.html]
[test_bug1823.html]

Просмотреть файл

@ -0,0 +1,131 @@
<!doctype html>
<title>Test for HTMLInputElement.lastInteractiveValue</title>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<script src="/tests/SimpleTest/NativeKeyCodes.js"></script>
<link href="/tests/SimpleTest/test.css"/>
<body>
<script>
const kIsMac = navigator.platform.indexOf("Mac") > -1;
const kIsWin = navigator.platform.indexOf("Win") > -1;
function getFreshInput() {
let input = document.body.appendChild(document.createElement("input"));
input.focus();
return input;
}
// XXX This should be add_setup, but bug 1776589
add_task(async function ensure_focus() {
await SimpleTest.promiseFocus(window);
});
add_task(async function simple() {
let input = getFreshInput();
is(SpecialPowers.wrap(input).lastInteractiveValue, "", "Initial state");
sendString("abc");
is(input.value, "abc", ".value after interactive edit");
is(SpecialPowers.wrap(input).lastInteractiveValue, "abc", ".lastInteractiveValue after interactive edit");
input.value = "muahahaha";
is(input.value, "muahahaha", ".value after script edit");
is(SpecialPowers.wrap(input).lastInteractiveValue, "abc", ".lastInteractiveValue after script edit");
});
add_task(async function test_default_value() {
let input = getFreshInput();
input.defaultValue = "default value";
is(input.value, "default value", ".defaultValue affects .value");
is(SpecialPowers.wrap(input).lastInteractiveValue, "", "Default value is not interactive");
sendString("abc");
is(SpecialPowers.wrap(input).lastInteractiveValue, "default valueabc", "After interaction with default value");
});
// This happens in imdb.com login form.
add_task(async function clone_after_interactive_edit() {
let input = getFreshInput();
sendString("abc");
is(input.value, "abc", ".value after interactive edit");
is(SpecialPowers.wrap(input).lastInteractiveValue, "abc", ".lastInteractiveValue after interactive edit");
let clone = input.cloneNode(true);
is(clone.value, "abc", ".value after clone");
is(SpecialPowers.wrap(clone).lastInteractiveValue, "abc", ".lastInteractiveValue after clone");
clone.type = "hidden";
clone.value = "something random";
is(SpecialPowers.wrap(clone).lastInteractiveValue, "", ".lastInteractiveValue after clone in non-text-input");
});
add_task(async function set_user_input() {
let input = getFreshInput();
input.value = "";
SpecialPowers.wrap(input).setUserInput("abc");
is(input.value, "abc", ".value after setUserInput edit");
is(SpecialPowers.wrap(input).lastInteractiveValue, "abc", ".lastInteractiveValue after setUserInput");
input.value = "foobar";
is(SpecialPowers.wrap(input).lastInteractiveValue, "abc", ".lastInteractiveValue after script edit after setUserInput");
});
// TODO(emilio): Maybe execCommand shouldn't be considered interactive, but it
// matches pre-existing behavior effectively.
add_task(async function exec_command() {
let input = getFreshInput();
document.execCommand("insertText", false, "a");
is(input.value, "a", ".value after execCommand edit");
is(SpecialPowers.wrap(input).lastInteractiveValue, "a", ".lastInteractiveValue after execCommand");
input.value = "foobar";
is(SpecialPowers.wrap(input).lastInteractiveValue, "a", ".lastInteractiveValue after script edit after execCommand");
});
add_task(async function cut_paste() {
if (!kIsMac && !kIsWin) {
todo(false, "synthesizeNativeKey doesn't work elsewhere (yet)");
return;
}
function doSynthesizeNativeKey(keyCode, modifiers, chars) {
return new Promise((resolve, reject) => {
if (!synthesizeNativeKey(KEYBOARD_LAYOUT_EN_US, keyCode, modifiers, chars, chars, resolve)) {
reject(new Error("Couldn't synthesize native key"));
}
});
}
let input = getFreshInput();
sendString("abc");
input.select();
is(SpecialPowers.wrap(input).lastInteractiveValue, "abc", ".lastInteractiveValue before cut");
await doSynthesizeNativeKey(kIsMac ? MAC_VK_ANSI_X : WIN_VK_X, { accelKey: true }, "x");
is(SpecialPowers.wrap(input).lastInteractiveValue, "", ".lastInteractiveValue after cut");
await doSynthesizeNativeKey(kIsMac ? MAC_VK_ANSI_V : WIN_VK_V, { accelKey: true }, "v");
is(SpecialPowers.wrap(input).lastInteractiveValue, "abc", ".lastInteractiveValue after paste");
});
</script>

Просмотреть файл

@ -185,6 +185,12 @@ partial interface HTMLInputElement {
[ChromeOnly]
attribute DOMString previewValue;
// Last value entered by the user, not by a script.
// NOTE(emilio): As of right now some execCommand triggered changes might be
// considered interactive.
[ChromeOnly]
readonly attribute DOMString lastInteractiveValue;
[ChromeOnly]
// This function will return null if @autocomplete is not defined for the
// current @type