INPUT_MULTIPLE_FIELDS_UI: Don't update shadow tree by updating any attribute
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Mar 2013 00:38:09 +0000 (00:38 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Mar 2013 00:38:09 +0000 (00:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=111990

Reviewed by Hajime Morrita.

Source/WebCore:

Bug detail:
Typing '1' then '1' into an hour field doesn't set the field to
'11' if an attribute is updated during these two keyboard inputs
because we always re-construct a shadow DOM tree by updating any
attributes and a shadow node records keyboard input state.

How to fix:
We should not re-construct a shadow DOM tree by updating
unaffected attributes. For example, we should re-construct it by
updating 'min' attribute, but we should not by updating 'class'
attribute.
We have InputType::updateInnerTextValue call in parseAttribute for
text field input types. The multiple fields input types
re-construct shadow DOM tree in updateInnerTextValue. The
updateInnerTextValue call is unnecessary for the multiple fields
input types, and we should call it when it is necessary. So, we
add InputType::attributeChange and text field input types call
updateInnerTextValue in it, and other input types don't.
Also, multiple fields input types need to call
updateInnerTextValue by 'value' attribute change. We add
InputType::valueAttributeChanged.

Tests: Update
fast/forms/time-multiple-fields/time-multiple-fields-keyboard-events.html. The
value attribute change behavior is covered by
time-multiple-fields-change-layout-by-value.html and
time-multiple-fields-spinbutton-change-and-input-events.html.

* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::parseAttribute):
- Add a valueAttributeChanged call.
- Calls InputType::attributeChanged
* html/InputType.cpp:
(WebCore::InputType::attributeChanged): Added an empty implementation.
(WebCore::InputType::valueAttributeChanged): Ditto.
* html/InputType.h:
(InputType): Declare attributeChanged and valueAttributeChanged.
* html/TextFieldInputType.cpp:
(WebCore::TextFieldInputType::attributeChanged):
Added. Always call updateInnerTextValue.
* html/TextFieldInputType.h:
(TextFieldInputType): Declare attributeChanged.
* html/BaseMultipleFieldsDateAndTimeInputType.cpp:
(WebCore::BaseMultipleFieldsDateAndTimeInputType::valueAttributeChanged):
Added. Re-construct shadow DOM tree if the input has no dirty value.
* html/BaseMultipleFieldsDateAndTimeInputType.h:
(BaseMultipleFieldsDateAndTimeInputType): Declare valueAttributeChanged.

LayoutTests:

* fast/forms/time-multiple-fields/time-multiple-fields-keyboard-events.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@146029 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/forms/time-multiple-fields/time-multiple-fields-keyboard-events.html
Source/WebCore/ChangeLog
Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp
Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.h
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/InputType.cpp
Source/WebCore/html/InputType.h
Source/WebCore/html/TextFieldInputType.cpp
Source/WebCore/html/TextFieldInputType.h

index d8e2f14..e583d93 100644 (file)
@@ -1,3 +1,12 @@
+2013-03-17  Kent Tamura  <tkent@chromium.org>
+
+        INPUT_MULTIPLE_FIELDS_UI: Don't update shadow tree by updating any attribute
+        https://bugs.webkit.org/show_bug.cgi?id=111990
+
+        Reviewed by Hajime Morrita.
+
+        * fast/forms/time-multiple-fields/time-multiple-fields-keyboard-events.html:
+
 2013-03-17  Rafael Weinstein  <rafaelw@chromium.org>
 
         [HTMLTemplateElement] prevent </template> from matching "template" in a non-HTML tags on the stack of open elements
index 3f9c31f..50f949a 100644 (file)
@@ -59,6 +59,7 @@ input.addEventListener('change', countEvents, false);
 beginTest('Digit keys');
 keyDown('7'); // -> 07:[--] --
 keyDown('5'); // -> 07:[05] --
+input.className = "CLASS";
 keyDown('6'); // -> 07:56 [--]
 shouldBeUndefined('eventsCounter.input');
 shouldBeUndefined('eventsCounter.change');
index 1f0ca04..761a591 100644 (file)
@@ -1,3 +1,58 @@
+2013-03-17  Kent Tamura  <tkent@chromium.org>
+
+        INPUT_MULTIPLE_FIELDS_UI: Don't update shadow tree by updating any attribute
+        https://bugs.webkit.org/show_bug.cgi?id=111990
+
+        Reviewed by Hajime Morrita.
+
+        Bug detail:
+        Typing '1' then '1' into an hour field doesn't set the field to
+        '11' if an attribute is updated during these two keyboard inputs
+        because we always re-construct a shadow DOM tree by updating any
+        attributes and a shadow node records keyboard input state.
+
+        How to fix:
+        We should not re-construct a shadow DOM tree by updating
+        unaffected attributes. For example, we should re-construct it by
+        updating 'min' attribute, but we should not by updating 'class'
+        attribute.
+        We have InputType::updateInnerTextValue call in parseAttribute for
+        text field input types. The multiple fields input types
+        re-construct shadow DOM tree in updateInnerTextValue. The
+        updateInnerTextValue call is unnecessary for the multiple fields
+        input types, and we should call it when it is necessary. So, we
+        add InputType::attributeChange and text field input types call
+        updateInnerTextValue in it, and other input types don't.
+        Also, multiple fields input types need to call
+        updateInnerTextValue by 'value' attribute change. We add
+        InputType::valueAttributeChanged.
+
+        Tests: Update
+        fast/forms/time-multiple-fields/time-multiple-fields-keyboard-events.html. The
+        value attribute change behavior is covered by
+        time-multiple-fields-change-layout-by-value.html and
+        time-multiple-fields-spinbutton-change-and-input-events.html.
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::parseAttribute):
+        - Add a valueAttributeChanged call.
+        - Calls InputType::attributeChanged
+        * html/InputType.cpp:
+        (WebCore::InputType::attributeChanged): Added an empty implementation.
+        (WebCore::InputType::valueAttributeChanged): Ditto.
+        * html/InputType.h:
+        (InputType): Declare attributeChanged and valueAttributeChanged.
+        * html/TextFieldInputType.cpp:
+        (WebCore::TextFieldInputType::attributeChanged):
+        Added. Always call updateInnerTextValue.
+        * html/TextFieldInputType.h:
+        (TextFieldInputType): Declare attributeChanged.
+        * html/BaseMultipleFieldsDateAndTimeInputType.cpp:
+        (WebCore::BaseMultipleFieldsDateAndTimeInputType::valueAttributeChanged):
+        Added. Re-construct shadow DOM tree if the input has no dirty value.
+        * html/BaseMultipleFieldsDateAndTimeInputType.h:
+        (BaseMultipleFieldsDateAndTimeInputType): Declare valueAttributeChanged.
+
 2013-03-17  Rafael Weinstein  <rafaelw@chromium.org>
 
         [HTMLTemplateElement] prevent </template> from matching "template" in a non-HTML tags on the stack of open elements
index f385a8d..f31af2d 100644 (file)
@@ -417,6 +417,12 @@ void BaseMultipleFieldsDateAndTimeInputType::updateInnerTextValue()
     updateClearButtonVisibility();
 }
 
+void BaseMultipleFieldsDateAndTimeInputType::valueAttributeChanged()
+{
+    if (!element()->hasDirtyValue())
+        updateInnerTextValue();
+}
+
 #if ENABLE(DATALIST_ELEMENT)
 void BaseMultipleFieldsDateAndTimeInputType::listAttributeTargetChanged()
 {
index fe0b84c..84483ad 100644 (file)
@@ -105,6 +105,7 @@ private:
     virtual bool shouldUseInputMethod() const OVERRIDE FINAL;
     virtual void stepAttributeChanged() OVERRIDE FINAL;
     virtual void updateInnerTextValue() OVERRIDE FINAL;
+    virtual void valueAttributeChanged() OVERRIDE;
     virtual void listAttributeTargetChanged() OVERRIDE FINAL;
     virtual void updateClearButtonVisibility() OVERRIDE FINAL;
 
index 8acabe6..180b6f8 100644 (file)
@@ -665,6 +665,7 @@ void HTMLInputElement::parseAttribute(const QualifiedName& name, const AtomicStr
         setFormControlValueMatchesRenderer(false);
         setNeedsValidityCheck();
         m_valueAttributeWasUpdatedAfterParsing = !m_parsingInProgress;
+        m_inputType->valueAttributeChanged();
     } else if (name == checkedAttr) {
         // Another radio button in the same group might be checked by state
         // restore. We shouldn't call setChecked() even if this has the checked
@@ -773,7 +774,7 @@ void HTMLInputElement::parseAttribute(const QualifiedName& name, const AtomicStr
 #endif
     else
         HTMLTextFormControlElement::parseAttribute(name, value);
-    m_inputType->updateInnerTextValue();
+    m_inputType->attributeChanged();
 }
 
 void HTMLInputElement::finishParsingChildren()
index 13fa96d..2f77cf2 100644 (file)
@@ -896,6 +896,10 @@ void InputType::updatePlaceholderText()
 {
 }
 
+void InputType::attributeChanged()
+{
+}
+
 void InputType::multipleAttributeChanged()
 {
 }
@@ -912,6 +916,10 @@ void InputType::requiredAttributeChanged()
 {
 }
 
+void InputType::valueAttributeChanged()
+{
+}
+
 void InputType::subtreeHasChanged()
 {
     ASSERT_NOT_REACHED();
index 8e6e534..3fd531e 100644 (file)
@@ -271,10 +271,12 @@ public:
     virtual bool supportsReadOnly() const;
     virtual void updateInnerTextValue();
     virtual void updatePlaceholderText();
+    virtual void attributeChanged();
     virtual void multipleAttributeChanged();
     virtual void disabledAttributeChanged();
     virtual void readonlyAttributeChanged();
     virtual void requiredAttributeChanged();
+    virtual void valueAttributeChanged();
     virtual String defaultToolTip() const;
 #if ENABLE(DATALIST_ELEMENT)
     virtual void listAttributeTargetChanged();
index e879baf..1a7fd07 100644 (file)
@@ -316,6 +316,13 @@ void TextFieldInputType::destroyShadowSubtree()
     m_container.clear();
 }
 
+void TextFieldInputType::attributeChanged()
+{
+    // FIXME: Updating the inner text on any attribute update should
+    // be unnecessary. We should figure out what attributes affect.
+    updateInnerTextValue();
+}
+
 void TextFieldInputType::disabledAttributeChanged()
 {
     if (m_innerSpinButton)
index fd73140..91c83bc 100644 (file)
@@ -63,6 +63,7 @@ protected:
     virtual bool shouldHaveSpinButton() const;
     virtual void createShadowSubtree() OVERRIDE;
     virtual void destroyShadowSubtree() OVERRIDE;
+    virtual void attributeChanged() OVERRIDE;
     virtual void disabledAttributeChanged() OVERRIDE;
     virtual void readonlyAttributeChanged() OVERRIDE;
     virtual bool supportsReadOnly() const OVERRIDE;