Skip to content

Commit

Permalink
Element::parserSetAttributes shouldn't trigger shadow tree constructi…
Browse files Browse the repository at this point in the history
…on in TextFieldInputType::attributeChanged

https://bugs.webkit.org/show_bug.cgi?id=267936

Reviewed by Aditya Keerthi and Yusuke Suzuki.

Don't construct shadow tree in TextFieldInputType::attributeChanged.

* Source/WebCore/html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::updateType): Sanitize the value before calling createShadowSubtreeIfNeeded. Otherwise,
we can assert in NumberInputType::localizeValue for example.

* Source/WebCore/html/HTMLTextFormControlElement.cpp:
(WebCore::HTMLTextFormControlElement::setSelectionRange):

* Source/WebCore/html/TextFieldInputType.cpp:
(WebCore::TextFieldInputType::isEmptyValue const): Now checks visibleValue() when the shadow tree doesn't exist yet.
(WebCore::TextFieldInputType::createShadowSubtree): Update the inner text value after creating the shadow tree.
(WebCore::TextFieldInputType::attributeChanged): Fixed the code so that it no longer constructs the shadow tree.

* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::visiblePlaceholder): Update the style / layout so that the shadow tree will get created.

Canonical link: https://commits.webkit.org/273461@main
  • Loading branch information
rniwa committed Jan 25, 2024
1 parent 22343e6 commit 66bf68c
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 10 deletions.
9 changes: 4 additions & 5 deletions Source/WebCore/html/HTMLInputElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,10 @@ void HTMLInputElement::updateType(const AtomString& typeAttributeValue)
bool previouslySelectable = m_inputType->supportsSelectionAPI();

m_inputType = WTFMove(newType);
if (!didStoreValue && willStoreValue)
m_valueIfDirty = sanitizeValue(attributeWithoutSynchronization(valueAttr));
else
updateValueIfNeeded();
m_inputType->createShadowSubtreeIfNeeded();

// https://html.spec.whatwg.org/multipage/dom.html#auto-directionality
Expand All @@ -560,11 +564,6 @@ void HTMLInputElement::updateType(const AtomString& typeAttributeValue)

updateWillValidateAndValidity();

if (!didStoreValue && willStoreValue)
m_valueIfDirty = sanitizeValue(attributeWithoutSynchronization(valueAttr));
else
updateValueIfNeeded();

setFormControlValueMatchesRenderer(false);
m_inputType->updateInnerTextValue();

Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/html/HTMLTextFormControlElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,13 @@ bool HTMLTextFormControlElement::setSelectionRange(unsigned start, unsigned end,
if (!isTextField())
return false;

auto innerText = innerTextElementCreatingShadowSubtreeIfNeeded();

// Clamps to the current value length.
unsigned innerTextValueLength = innerTextValue().length();
end = std::min(end, innerTextValueLength);
start = std::min(start, end);

auto innerText = innerTextElementCreatingShadowSubtreeIfNeeded();
bool hasFocus = document().focusedElement() == this;

RefPtr frame = document().frame();
Expand Down
10 changes: 6 additions & 4 deletions Source/WebCore/html/TextFieldInputType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ bool TextFieldInputType::isEmptyValue() const
{
auto innerText = innerTextElement();
if (!innerText) {
// Since we always create the shadow subtree if a value is set, we know
// that the value is empty.
return true;
if (element()->shadowRoot())
return true; // Shadow tree is empty
return visibleValue().isEmpty();
}

for (Text* text = TextNodeTraversal::firstWithin(*innerText); text; text = TextNodeTraversal::next(*text, innerText.get())) {
Expand Down Expand Up @@ -357,11 +357,13 @@ void TextFieldInputType::createShadowSubtree()
if (!createsContainer) {
element()->userAgentShadowRoot()->appendChild(ContainerNode::ChildChange::Source::Parser, *m_innerText);
updatePlaceholderText();
updateInnerTextValue();
return;
}

createContainer(PreserveSelectionRange::No);
updatePlaceholderText();
updateInnerTextValue();

if (shouldHaveSpinButton) {
m_innerSpinButton = SpinButtonElement::create(document, *this);
Expand Down Expand Up @@ -435,7 +437,7 @@ void TextFieldInputType::removeShadowSubtree()
void TextFieldInputType::attributeChanged(const QualifiedName& name)
{
if (name == valueAttr || name == placeholderAttr) {
if (element())
if (element() && element()->shadowRoot())
updateInnerTextValue();
}
InputType::attributeChanged(name);
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/testing/Internals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,8 @@ std::optional<Internals::EventThrottlingBehavior> Internals::eventThrottlingBeha

String Internals::visiblePlaceholder(Element& element)
{
element.document().updateLayout(LayoutOptions::IgnorePendingStylesheets);

if (is<HTMLTextFormControlElement>(element)) {
const HTMLTextFormControlElement& textFormControlElement = downcast<HTMLTextFormControlElement>(element);
if (!textFormControlElement.isPlaceholderVisible())
Expand Down

0 comments on commit 66bf68c

Please sign in to comment.