Commit 0ca2455d authored by Kent Tamura's avatar Kent Tamura Committed by Commit Bot

Revert "Restoring control state should dispatch 'input' and 'change' events...

Revert "Restoring control state should dispatch 'input' and 'change' events only if values are changed"

This reverts commit ca02d552.

Reason for revert: It was a wrong approach. https://bugs.chromium.org/p/chromium/issues/detail?id=1037728#c11

Original change's description:
> Restoring control state should dispatch 'input' and 'change' events only if values are changed
> 
> This CL is a follow-up of crrev.com/727843, which caused a regression
> seen in the local-NTP.
> 
> * HTMLTextAreaElement and HTMLSelectElement:
>   Call QueueInputAndChangeEvents() only if their values are changed.
> 
> * HTMLInputElement and input type implementations:
>   InputTypeView::RestoreFormControlState() returns a boolean flag to ask to
>   call QueueInputAndChangeEvents().
> 
> Bug: 1038655
> Change-Id: Ib68077516d1d6c032dbc2599abbcd541c281f4de
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1983692
> Commit-Queue: Kent Tamura <tkent@chromium.org>
> Reviewed-by: Mason Freed <masonfreed@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#729334}

TBR=tkent@chromium.org,masonfreed@chromium.org

Change-Id: I645ae24b3feaa2a29f26812f360fe87f46a523e6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1038655
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1993077Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729709}
parent 4da102a4
...@@ -55,11 +55,9 @@ FormControlState BaseCheckableInputType::SaveFormControlState() const { ...@@ -55,11 +55,9 @@ FormControlState BaseCheckableInputType::SaveFormControlState() const {
return FormControlState(GetElement().checked() ? "on" : "off"); return FormControlState(GetElement().checked() ? "on" : "off");
} }
bool BaseCheckableInputType::RestoreFormControlState( void BaseCheckableInputType::RestoreFormControlState(
const FormControlState& state) { const FormControlState& state) {
bool old_checked = GetElement().checked();
GetElement().setChecked(state[0] == "on"); GetElement().setChecked(state[0] == "on");
return old_checked != GetElement().checked();
} }
void BaseCheckableInputType::AppendToFormData(FormData& form_data) const { void BaseCheckableInputType::AppendToFormData(FormData& form_data) const {
......
...@@ -57,7 +57,7 @@ class BaseCheckableInputType : public InputType, public InputTypeView { ...@@ -57,7 +57,7 @@ class BaseCheckableInputType : public InputType, public InputTypeView {
private: private:
InputTypeView* CreateView() override; InputTypeView* CreateView() override;
FormControlState SaveFormControlState() const final; FormControlState SaveFormControlState() const final;
bool RestoreFormControlState(const FormControlState&) final; void RestoreFormControlState(const FormControlState&) final;
void AppendToFormData(FormData&) const final; void AppendToFormData(FormData&) const final;
void HandleKeypressEvent(KeyboardEvent&) final; void HandleKeypressEvent(KeyboardEvent&) final;
bool CanSetStringValue() const final; bool CanSetStringValue() const final;
......
...@@ -116,11 +116,9 @@ FormControlState FileInputType::SaveFormControlState() const { ...@@ -116,11 +116,9 @@ FormControlState FileInputType::SaveFormControlState() const {
return state; return state;
} }
bool FileInputType::RestoreFormControlState(const FormControlState& state) { void FileInputType::RestoreFormControlState(const FormControlState& state) {
// Check if the state is broken.
// See File::AppendToControlState() and File::CreateFromControlState().
if (state.ValueSize() % 3) if (state.ValueSize() % 3)
return false; return;
HeapVector<Member<File>> file_vector = HeapVector<Member<File>> file_vector =
CreateFilesFrom<File*, HeapVector<Member<File>>>( CreateFilesFrom<File*, HeapVector<Member<File>>>(
state, &File::CreateFromControlState); state, &File::CreateFromControlState);
...@@ -128,9 +126,6 @@ bool FileInputType::RestoreFormControlState(const FormControlState& state) { ...@@ -128,9 +126,6 @@ bool FileInputType::RestoreFormControlState(const FormControlState& state) {
for (const auto& file : file_vector) for (const auto& file : file_vector)
file_list->Append(file); file_list->Append(file);
SetFilesAndDispatchEvents(file_list); SetFilesAndDispatchEvents(file_list);
// This function always returns false because SetFilesAndDispatchEvents()
// dispatches events if files are changed.
return false;
} }
void FileInputType::AppendToFormData(FormData& form_data) const { void FileInputType::AppendToFormData(FormData& form_data) const {
......
...@@ -67,7 +67,7 @@ class CORE_EXPORT FileInputType final : public InputType, ...@@ -67,7 +67,7 @@ class CORE_EXPORT FileInputType final : public InputType,
InputTypeView* CreateView() override; InputTypeView* CreateView() override;
const AtomicString& FormControlType() const override; const AtomicString& FormControlType() const override;
FormControlState SaveFormControlState() const override; FormControlState SaveFormControlState() const override;
bool RestoreFormControlState(const FormControlState&) override; void RestoreFormControlState(const FormControlState&) override;
void AppendToFormData(FormData&) const override; void AppendToFormData(FormData&) const override;
bool ValueMissing(const String&) const override; bool ValueMissing(const String&) const override;
String ValueMissingText() const override; String ValueMissingText() const override;
......
...@@ -584,10 +584,9 @@ FormControlState HTMLInputElement::SaveFormControlState() const { ...@@ -584,10 +584,9 @@ FormControlState HTMLInputElement::SaveFormControlState() const {
} }
void HTMLInputElement::RestoreFormControlState(const FormControlState& state) { void HTMLInputElement::RestoreFormControlState(const FormControlState& state) {
if (input_type_view_->RestoreFormControlState(state)) { input_type_view_->RestoreFormControlState(state);
state_restored_ = true; state_restored_ = true;
QueueInputAndChangeEvents(); QueueInputAndChangeEvents();
}
} }
bool HTMLInputElement::CanStartSelection() const { bool HTMLInputElement::CanStartSelection() const {
......
...@@ -1187,13 +1187,6 @@ void HTMLSelectElement::RestoreFormControlState(const FormControlState& state) { ...@@ -1187,13 +1187,6 @@ void HTMLSelectElement::RestoreFormControlState(const FormControlState& state) {
if (items_size == 0) if (items_size == 0)
return; return;
Vector<bool> old_selection(items_size);
for (wtf_size_t i = 0; i < items_size; ++i) {
if (auto* option = DynamicTo<HTMLOptionElement>(items[i].Get()))
old_selection[i] = option->Selected();
else
old_selection[i] = false;
}
SelectOption(nullptr, kDeselectOtherOptionsFlag); SelectOption(nullptr, kDeselectOtherOptionsFlag);
// The saved state should have at least one value and an index. // The saved state should have at least one value and an index.
...@@ -1244,15 +1237,7 @@ void HTMLSelectElement::RestoreFormControlState(const FormControlState& state) { ...@@ -1244,15 +1237,7 @@ void HTMLSelectElement::RestoreFormControlState(const FormControlState& state) {
} }
SetNeedsValidityCheck(); SetNeedsValidityCheck();
QueueInputAndChangeEvents();
for (wtf_size_t i = 0; i < items_size; ++i) {
if (auto* option = DynamicTo<HTMLOptionElement>(items[i].Get())) {
if (old_selection[i] != option->Selected()) {
QueueInputAndChangeEvents();
break;
}
}
}
} }
void HTMLSelectElement::ParseMultipleAttribute(const AtomicString& value) { void HTMLSelectElement::ParseMultipleAttribute(const AtomicString& value) {
......
...@@ -94,8 +94,6 @@ FormControlState HTMLTextAreaElement::SaveFormControlState() const { ...@@ -94,8 +94,6 @@ FormControlState HTMLTextAreaElement::SaveFormControlState() const {
void HTMLTextAreaElement::RestoreFormControlState( void HTMLTextAreaElement::RestoreFormControlState(
const FormControlState& state) { const FormControlState& state) {
if (EqualIgnoringNullity(value(), state[0]))
return;
// We don't add kDispatchInputAndChangeEvent to setValue(), and we // We don't add kDispatchInputAndChangeEvent to setValue(), and we
// post tasks to dispatch events instead. This function can be called // post tasks to dispatch events instead. This function can be called
// while we should not dispatch any events. // while we should not dispatch any events.
......
...@@ -177,10 +177,8 @@ FormControlState InputTypeView::SaveFormControlState() const { ...@@ -177,10 +177,8 @@ FormControlState InputTypeView::SaveFormControlState() const {
return FormControlState(current_value); return FormControlState(current_value);
} }
bool InputTypeView::RestoreFormControlState(const FormControlState& state) { void InputTypeView::RestoreFormControlState(const FormControlState& state) {
String old_value = GetElement().value();
GetElement().setValue(state[0]); GetElement().setValue(state[0]);
return !EqualIgnoringNullity(old_value, GetElement().value());
} }
bool InputTypeView::HasBadInput() const { bool InputTypeView::HasBadInput() const {
......
...@@ -128,9 +128,7 @@ class CORE_EXPORT InputTypeView : public GarbageCollectedMixin { ...@@ -128,9 +128,7 @@ class CORE_EXPORT InputTypeView : public GarbageCollectedMixin {
virtual void EnsurePrimaryContent() {} virtual void EnsurePrimaryContent() {}
virtual bool HasFallbackContent() const { return false; } virtual bool HasFallbackContent() const { return false; }
virtual FormControlState SaveFormControlState() const; virtual FormControlState SaveFormControlState() const;
// Should return |true| if the value is changed and the callsite needs to virtual void RestoreFormControlState(const FormControlState&);
// dispatch 'input' and 'change' events.
virtual bool RestoreFormControlState(const FormControlState&);
// Validation functions // Validation functions
virtual bool HasBadInput() const; virtual bool HasBadInput() const;
......
...@@ -527,18 +527,16 @@ void MultipleFieldsTemporalInputTypeView::ReadonlyAttributeChanged() { ...@@ -527,18 +527,16 @@ void MultipleFieldsTemporalInputTypeView::ReadonlyAttributeChanged() {
edit->ReadOnlyStateChanged(); edit->ReadOnlyStateChanged();
} }
bool MultipleFieldsTemporalInputTypeView::RestoreFormControlState( void MultipleFieldsTemporalInputTypeView::RestoreFormControlState(
const FormControlState& state) { const FormControlState& state) {
DateTimeEditElement* edit = GetDateTimeEditElement(); DateTimeEditElement* edit = GetDateTimeEditElement();
if (!edit) if (!edit)
return false; return;
String old_value = GetElement().value();
DateTimeFieldsState date_time_fields_state = DateTimeFieldsState date_time_fields_state =
DateTimeFieldsState::RestoreFormControlState(state); DateTimeFieldsState::RestoreFormControlState(state);
edit->SetValueAsDateTimeFieldsState(date_time_fields_state); edit->SetValueAsDateTimeFieldsState(date_time_fields_state);
GetElement().SetNonAttributeValue(input_type_->SanitizeValue(edit->Value())); GetElement().SetNonAttributeValue(input_type_->SanitizeValue(edit->Value()));
UpdateClearButtonVisibility(); UpdateClearButtonVisibility();
return !EqualIgnoringNullity(old_value, GetElement().value());
} }
FormControlState MultipleFieldsTemporalInputTypeView::SaveFormControlState() FormControlState MultipleFieldsTemporalInputTypeView::SaveFormControlState()
......
...@@ -107,7 +107,7 @@ class MultipleFieldsTemporalInputTypeView final ...@@ -107,7 +107,7 @@ class MultipleFieldsTemporalInputTypeView final
void MinOrMaxAttributeChanged() final; void MinOrMaxAttributeChanged() final;
void ReadonlyAttributeChanged() final; void ReadonlyAttributeChanged() final;
void RequiredAttributeChanged() final; void RequiredAttributeChanged() final;
bool RestoreFormControlState(const FormControlState&) final; void RestoreFormControlState(const FormControlState&) final;
FormControlState SaveFormControlState() const final; FormControlState SaveFormControlState() const final;
void DidSetValue(const String&, bool value_changed) final; void DidSetValue(const String&, bool value_changed) final;
void StepAttributeChanged() final; void StepAttributeChanged() final;
......
...@@ -69,10 +69,9 @@ FormControlState PasswordInputType::SaveFormControlState() const { ...@@ -69,10 +69,9 @@ FormControlState PasswordInputType::SaveFormControlState() const {
return FormControlState(); return FormControlState();
} }
bool PasswordInputType::RestoreFormControlState(const FormControlState&) { void PasswordInputType::RestoreFormControlState(const FormControlState&) {
// Should never save/restore password fields. // Should never save/restore password fields.
NOTREACHED(); NOTREACHED();
return false;
} }
bool PasswordInputType::ShouldRespectListAttribute() { bool PasswordInputType::ShouldRespectListAttribute() {
......
...@@ -47,7 +47,7 @@ class PasswordInputType final : public BaseTextInputType { ...@@ -47,7 +47,7 @@ class PasswordInputType final : public BaseTextInputType {
const AtomicString& FormControlType() const override; const AtomicString& FormControlType() const override;
bool ShouldSaveAndRestoreFormControlState() const override; bool ShouldSaveAndRestoreFormControlState() const override;
FormControlState SaveFormControlState() const override; FormControlState SaveFormControlState() const override;
bool RestoreFormControlState(const FormControlState&) override; void RestoreFormControlState(const FormControlState&) override;
bool ShouldRespectListAttribute() override; bool ShouldRespectListAttribute() override;
bool NeedsContainer() const override; bool NeedsContainer() const override;
......
<!DOCTYPE html> <!DOCTYPE html>
<script> <script>
document.addEventListener('change', e => { document.addEventListener('change', e => {
parent.inputOrChangeEvents.push('change/' + e.target.id); parent.inputOrChangeEvents.push('change/' + e.target.type);
}); });
document.addEventListener('input', e => { document.addEventListener('input', e => {
parent.inputOrChangeEvents.push('input/' + e.target.id); parent.inputOrChangeEvents.push('input/' + e.target.type);
}); });
</script> </script>
<div></div> <div></div>
<input type=checkbox disabled id="checkbox-disabled"> <input type=checkbox disabled>
<input type=text disabled id="text-disabled"> <input type=text disabled>
<input type=checkbox id="checkbox-unmodified"> <textarea></textarea>
<input type=text id="text-unmodified"> <select><option>1<option>2</select>
<input type=file id="file-modified">
<input type=file id="file-unmodified">
<input type=date id="date-modified">
<input type=date id="date-unmodified">
<textarea id="textarea-modified"></textarea>
<textarea id="textarea-unmodified">abc
def</textarea>
<select id="select-modified"><option>1<option>2</select>
<select id="select-unmodified" multiple>
<option selected>1</option>
<option>2</option>
<option selected>3</option>
</select>
<script> <script>
document.querySelector('div').innerHTML = '<input type=checkbox id="checkbox-modified"><input type=text id="text-modified">'; document.querySelector('div').innerHTML = '<input type=checkbox><input type=text>'
</script> </script>
...@@ -2,17 +2,6 @@ ...@@ -2,17 +2,6 @@
<script src="../../resources/testharness.js"></script> <script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script> <script src="../../resources/testharnessreport.js"></script>
<script src="../../external/wpt/html/interaction/focus/the-autofocus-attribute/resources/utils.js"></script> <script src="../../external/wpt/html/interaction/focus/the-autofocus-attribute/resources/utils.js"></script>
<script src="file/resources/file-drag-common.js"></script>
<style>
/* Make the top-left corner of the iframe is equal to the top-left corner
of the body. It's required for dragFilesOntoInput(). */
body {
margin: 0;
}
iframe {
border: none;
}
</style>
<body> <body>
<iframe src="resources/state-restore-dynamic-controls-frame.html"></iframe> <iframe src="resources/state-restore-dynamic-controls-frame.html"></iframe>
<script> <script>
...@@ -27,16 +16,14 @@ promise_test(async t => { ...@@ -27,16 +16,14 @@ promise_test(async t => {
container.firstChild.click(); container.firstChild.click();
container.lastChild.focus(); container.lastChild.focus();
eventSender.keyDown('z'); eventSender.keyDown('z');
dragFilesOntoInput(doc.querySelector('#file-modified'), ['foo.txt']); doc.querySelector('textarea').focus();
doc.querySelector('#date-modified').value = '2020-01-08';
doc.querySelector('#textarea-modified').focus();
eventSender.keyDown('y'); eventSender.keyDown('y');
doc.querySelector('#select-modified').focus(); doc.querySelector('select').focus();
eventSender.keyDown('2'); eventSender.keyDown('2');
assert_true(container.firstChild.checked, 'sanity check for a checkbox'); assert_true(container.firstChild.checked, 'sanity check for a checkbox');
assert_equals(container.lastChild.value, 'z', 'sanity check for a text field'); assert_equals(container.lastChild.value, 'z', 'sanity check for a text field');
assert_equals(doc.querySelector('#textarea-modified').value, 'y', 'sanity check for a textarea'); assert_equals(doc.querySelector('textarea').value, 'y', 'sanity check for a textarea');
assert_equals(doc.querySelector('#select-modified').value, '2', 'sanity check for a select'); assert_equals(doc.querySelector('select').value, '2', 'sanity check for a select');
// Flush asynchronous input/change events // Flush asynchronous input/change events
await timeOut(t, 0); await timeOut(t, 0);
...@@ -63,11 +50,7 @@ promise_test(async t => { ...@@ -63,11 +50,7 @@ promise_test(async t => {
// Wait for asynchronous input/change events // Wait for asynchronous input/change events
await timeOut(t, 0); await timeOut(t, 0);
assert_array_equals(inputOrChangeEvents, [ assert_array_equals(inputOrChangeEvents, [
'input/file-modified', 'change/file-modified', 'input/checkbox', 'change/checkbox', 'input/text', 'change/text',
'input/checkbox-modified', 'change/checkbox-modified', 'input/textarea', 'change/textarea', 'input/select-one', 'change/select-one']);
'input/text-modified', 'change/text-modified',
'input/date-modified', 'change/date-modified',
'input/textarea-modified', 'change/textarea-modified',
'input/select-modified', 'change/select-modified']);
}, 'Control states should be restored correctly even if controls were inserted before existing controls.'); }, 'Control states should be restored correctly even if controls were inserted before existing controls.');
</script> </script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment