Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-08 19:04:45 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1833477 - Make type attribute changes follow the spec more closely. r=smaug,masayuki

Browse Source 
The issue here is that the SetValueInternal call made us go from empty
value to non-empty due to color's specialness of returning black always,
but since the value dirty flag wasn't set, the non-empty value actually
didn't make a difference and just confused the code.

Make the code follow the spec more closely, which fixes this (in
particular the SetDefaultValueAsValue call).

Also, remove some useless code in SetValueInternal() since callers end
up in OnValueChanged() effectively everywhere except when mDoneCreating
is false (in which case we can just wait until DoneCreatingElement calls
us again), and if the do not end up there then that's a bug to fix.

Differential Revision: https://phabricator.services.mozilla.com/D178706
This commit is contained in:
Emilio Cobos Álvarez 2023-05-23 15:18:03 +00:00
parent 24d941f4a5
commit b8ac0dfcbe
4 changed files with 108 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

159
dom/html/HTMLInputElement.cpp
View File

@ -2658,12 +2658,6 @@ nsresult HTMLInputElement::SetValueInternal(
SetValueChanged(true);
}
if (value.IsEmpty()) {
AddStates(ElementState::VALUE_EMPTY);
} else {
RemoveStates(ElementState::VALUE_EMPTY);
}
if (IsSingleLineTextControl(false)) {
// Note that if aOptions includes
// ValueSetterOption::BySetUserInputAPI, "input" event is automatically
@ -2720,15 +2714,6 @@ nsresult HTMLInputElement::SetValueInternal(
colorControlFrame->UpdateColor();
}
}
// This call might be useless in some situations because if the element is
// a single line text control, TextControlState::SetValue will call
// HTMLInputElement::OnValueChanged which is going to call UpdateState()
// if the element is focused. This bug 665547.
if (PlaceholderApplies() && HasAttr(nsGkAtoms::placeholder)) {
UpdateState(true);
}
return NS_OK;
}
@ -4307,48 +4292,18 @@ void HTMLInputElement::UnbindFromTree(bool aNullParent) {
UpdateState(false);
}
namespace {
class TypeChangeSelectionRangeFlagDeterminer {
public:
using ValueSetterOption = TextControlState::ValueSetterOption;
using ValueSetterOptions = TextControlState::ValueSetterOptions;
// @param aOldType InputElementTypes
// @param aNewType InputElementTypes
TypeChangeSelectionRangeFlagDeterminer(FormControlType aOldType,
FormControlType aNewType)
: mOldType(aOldType), mNewType(aNewType) {}
// @return TextControlState::ValueSetterOptions
ValueSetterOptions GetValueSetterOptions() const {
const bool previouslySelectable = DoesSetRangeTextApply(mOldType);
const bool nowSelectable = DoesSetRangeTextApply(mNewType);
const bool moveCursorToBeginAndSetDirectionForward =
!previouslySelectable && nowSelectable;
if (moveCursorToBeginAndSetDirectionForward) {
return {ValueSetterOption::MoveCursorToBeginSetSelectionDirectionForward};
}
return {};
}
private:
/**
* @param aType InputElementTypes
* @return true, iff SetRangeText applies to aType as specified at
* https://html.spec.whatwg.org/multipage/input.html#concept-input-apply.
*/
static bool DoesSetRangeTextApply(FormControlType aType) {
return aType == FormControlType::InputText ||
aType == FormControlType::InputSearch ||
aType == FormControlType::InputUrl ||
aType == FormControlType::InputTel ||
aType == FormControlType::InputPassword;
}
const FormControlType mOldType;
const FormControlType mNewType;
};
} // anonymous namespace
/**
* @param aType InputElementTypes
* @return true, iff SetRangeText applies to aType as specified at
* https://html.spec.whatwg.org/#concept-input-apply.
*/
static bool SetRangeTextApplies(FormControlType aType) {
return aType == FormControlType::InputText ||
aType == FormControlType::InputSearch ||
aType == FormControlType::InputUrl ||
aType == FormControlType::InputTel ||
aType == FormControlType::InputPassword;
}
void HTMLInputElement::HandleTypeChange(FormControlType aNewType,
bool aNotify) {
@ -4385,13 +4340,12 @@ void HTMLInputElement::HandleTypeChange(FormControlType aNewType,
CancelRangeThumbDrag(false);
}
ValueModeType aOldValueMode = GetValueMode();
nsAutoString aOldValue;
if (aOldValueMode == VALUE_MODE_VALUE) {
const ValueModeType oldValueMode = GetValueMode();
nsAutoString oldValue;
if (oldValueMode == VALUE_MODE_VALUE) {
// Doesn't matter what caller type we pass here, since we know we're not a
// file input anyway.
GetValue(aOldValue, CallerType::NonSystem);
GetValue(oldValue, CallerType::NonSystem);
}
TextControlState::SelectionProperties sp;
@ -4414,47 +4368,64 @@ void HTMLInputElement::HandleTypeChange(FormControlType aNewType,
}
}
/**
* The following code is trying to reproduce the algorithm described here:
* http://www.whatwg.org/specs/web-apps/current-work/complete.html#input-type-change
*/
// https://html.spec.whatwg.org/#input-type-change
switch (GetValueMode()) {
case VALUE_MODE_DEFAULT:
case VALUE_MODE_DEFAULT_ON:
// If the previous value mode was value, we need to set the value content
// attribute to the previous value.
// There is no value sanitizing algorithm for elements in this mode.
if (aOldValueMode == VALUE_MODE_VALUE && !aOldValue.IsEmpty()) {
SetAttr(kNameSpaceID_None, nsGkAtoms::value, aOldValue, true);
// 1. If the previous state of the element's type attribute put the value
// IDL attribute in the value mode, and the element's value is not the
// empty string, and the new state of the element's type attribute puts
// the value IDL attribute in either the default mode or the default/on
// mode, then set the element's value content attribute to the
// element's value.
if (oldValueMode == VALUE_MODE_VALUE && !oldValue.IsEmpty()) {
SetAttr(kNameSpaceID_None, nsGkAtoms::value, oldValue, true);
}
break;
case VALUE_MODE_VALUE:
// If the previous value mode wasn't value, we have to set the value to
// the value content attribute.
// SetValueInternal is going to sanitize the value.
{
case VALUE_MODE_VALUE: {
ValueSetterOptions options{ValueSetterOption::ByInternalAPI};
if (!SetRangeTextApplies(oldType) && SetRangeTextApplies(mType)) {
options +=
ValueSetterOption::MoveCursorToBeginSetSelectionDirectionForward;
}
if (oldValueMode != VALUE_MODE_VALUE) {
// 2. Otherwise, if the previous state of the element's type attribute
// put the value IDL attribute in any mode other than the value
// mode, and the new state of the element's type attribute puts the
// value IDL attribute in the value mode, then set the value of the
// element to the value of the value content attribute, if there is
// one, or the empty string otherwise, and then set the control's
// dirty value flag to false.
nsAutoString value;
if (aOldValueMode != VALUE_MODE_VALUE) {
GetAttr(kNameSpaceID_None, nsGkAtoms::value, value);
} else {
value = aOldValue;
}
const TypeChangeSelectionRangeFlagDeterminer flagDeterminer(oldType,
mType);
ValueSetterOptions options(flagDeterminer.GetValueSetterOptions());
options += ValueSetterOption::ByInternalAPI;
GetAttr(nsGkAtoms::value, value);
SetValueInternal(value, options);
SetValueChanged(false);
} else if (mValueChanged) {
// We're both in the "value" mode state, we need to make no change per
// spec, but due to how we store the value internally we need to call
// SetValueInternal, if our value had changed at all.
// TODO: What should we do if SetValueInternal fails? (The allocation
// may potentially be big, but most likely we've failed to allocate
// before the type change.)
SetValueInternal(value, options);
SetValueInternal(oldValue, options);
} else {
// The value dirty flag is not set, so our value is based on our default
// value. But our default value might be dependent on the type. Make
// sure to set it so that state is consistent.
SetDefaultValueAsValue();
}
break;
}
case VALUE_MODE_FILENAME:
default:
// We don't care about the value.
// There is no value sanitizing algorithm for elements in this mode.
// 3. Otherwise, if the previous state of the element's type attribute
// put the value IDL attribute in any mode other than the filename
// mode, and the new state of the element's type attribute puts the
// value IDL attribute in the filename mode, then set the value of the
// element to the empty string.
//
// Setting the attribute to the empty string is basically calling
// ClearFiles, but there can't be any files.
break;
}
@ -5997,17 +5968,17 @@ void HTMLInputElement::DoneCreatingElement() {
// Sanitize the value and potentially set mFocusedValue.
if (GetValueMode() == VALUE_MODE_VALUE) {
nsAutoString aValue;
GetValue(aValue, CallerType::System);
nsAutoString value;
GetValue(value, CallerType::System);
// TODO: What should we do if SetValueInternal fails? (The allocation
// may potentially be big, but most likely we've failed to allocate
// before the type change.)
SetValueInternal(aValue, ValueSetterOption::ByInternalAPI);
SetValueInternal(value, ValueSetterOption::ByInternalAPI);
if (CreatesDateTimeWidget()) {
// mFocusedValue has to be set here, so that `FireChangeEventIfNeeded` can
// fire a change event if necessary.
mFocusedValue = aValue;
mFocusedValue = value;
}
}

2
dom/html/HTMLInputElement.h
View File

@ -1533,6 +1533,8 @@ class HTMLInputElement final : public TextControlElement,
nsContentUtils::AutocompleteAttrState mAutocompleteAttrState;
nsContentUtils::AutocompleteAttrState mAutocompleteInfoState;
bool mDisabledChanged : 1;
// https://html.spec.whatwg.org/#concept-fe-dirty
// TODO: Maybe rename to match the spec?
bool mValueChanged : 1;
bool mLastValueChangeWasInteractive : 1;
bool mCheckedChanged : 1;

8
testing/web-platform/tests/html/semantics/forms/the-input-element/input-type-change-empty-crash.html Normal file
View File

@ -0,0 +1,8 @@
<script>
document.addEventListener('DOMContentLoaded', () => {
a.type = "foo"
document.execCommand("insertHorizontalRule", false)
a.type = "text"
})
</script>
<input id="a" type="color">

33
testing/web-platform/tests/html/semantics/forms/the-input-element/input-type-change-value.html Normal file
View File

@ -0,0 +1,33 @@
<!doctype html>
<meta charset="utf-8">
<title>Input type switch from / to color</title>
<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
<link rel="author" href="https://mozilla.org" title="Mozilla">
<link rel="help" href="https://html.spec.whatwg.org/#input-type-change">
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1833477">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
function runTest(focus) {
let input = document.createElement("input");
input.type = "color";
document.body.appendChild(input);
if (focus) {
input.focus();
}
assert_equals(input.value, "#000000", "Invalid color should return a non-empty sanitized value");
input.type = "text";
assert_equals(input.value, "", "Value dirty flag should remain false");
input.type = "color";
input.value = "#ffffff";
assert_equals(input.value, "#ffffff", "Valid color is returned");
input.type = "text";
assert_equals(input.value, "#ffffff", "Value dirty flag should remain true");
if (focus) {
assert_equals(document.activeElement, input, "Focus is preserved");
}
}
test(() => runTest(false), "Without focus");
test(() => runTest(true), "With focus");
</script>