- recommit this change, since rolling it out did NOT fix the performance regression!
authorvicki <vicki@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Feb 2005 19:38:59 +0000 (19:38 +0000)
committervicki <vicki@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Feb 2005 19:38:59 +0000 (19:38 +0000)
    2005-02-23  Darin Adler  <darin@apple.com>

        Reviewed by John.

        - fixed <rdar://problem/4011405> REGRESSION (180-181): Unconfirmed text disappears when text focus moves

        The key was to change things around so that we don't push text from the DOM to the widget
        unless the DOM has actually been changed. This prevents the code path that wipes out inline input
        during the blur process.

        * khtml/html/html_formimpl.cpp:
        (DOM::HTMLInputElementImpl::HTMLInputElementImpl): Start m_valueMatchesRenderer as false.
        (DOM::HTMLInputElementImpl::parseHTMLAttribute): Set m_valueMatchesRenderer to false when a
        new value is set here.
        (DOM::HTMLInputElementImpl::setValue): Set m_valueMatchesRenderer to false when a new value
        is set here.
        (DOM::HTMLInputElementImpl::setValueFromRenderer): Added. Sets m_value, sets m_valueMatchesRenderer
        to true, and also sends out the input event. It's better to have this here than in the renderer code.
        (DOM::HTMLTextAreaElementImpl::HTMLTextAreaElementImpl): Start m_valueIsValid as false (replaces
        m_dirtyvalue) and m_valueMatchesRenderer as false.
        (DOM::HTMLTextAreaElementImpl::updateValue): Added. Factored this out from the value function. Uses
        the new booleans and keeps them up to date, specifically setting m_valueMatchesRenderer based on
        where the value came from.
        (DOM::HTMLTextAreaElementImpl::value): Updated to call updateValue to do most of the work.
        (DOM::HTMLTextAreaElementImpl::setValue): Set both m_valueIsValid and m_valueMatchesRenderer.
        (DOM::HTMLTextAreaElementImpl::setDefaultValue): Take parameter by reference.

        * khtml/html/html_formimpl.h: Added setValueFromRenderer, valueMatchesRenderer, setValueMatchesRenderer,
        and m_valueMatchesRenderer to input element. For textarea element, made some parameters pass DOMString
        by reference, and added invalidateValue, updateValue, valueMatchesRenderer, and setValueMatchesRenderer.

        * khtml/rendering/render_form.cpp:
        (RenderLineEdit::updateFromElement): Don't re-get the value from the DOM if valueMatchesRenderer
        is true.
        (RenderLineEdit::slotTextChanged): Call setValueFromRenderer instead of manipulating the DOM
        directly.
        (RenderTextArea::detach): Call updateValue instead of calling value for its side effect.
        (RenderTextArea::handleFocusOut): Ditto.
        (RenderTextArea::updateFromElement): Call updateValue and then not re-get the value from the
        DOM if valueMatchesRenderer is true.
        (RenderTextArea::slotTextChanged): Call invalidateValue instead of directly setting m_dirtyvalue to true.

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/html/html_formimpl.cpp
WebCore/khtml/html/html_formimpl.h
WebCore/khtml/rendering/render_form.cpp

index d9606be45146f358c8709cc7669fde11acc50323..9bbd9779e9fbb7715aa762b29521f7d35011cc3e 100644 (file)
@@ -1,3 +1,49 @@
+2005-02-25  Vicki Murley <vicki@apple.com>
+
+       - recommit this change, since rolling it out did NOT fix the performance regression!
+
+    2005-02-23  Darin Adler  <darin@apple.com>
+
+        Reviewed by John.
+
+        - fixed <rdar://problem/4011405> REGRESSION (180-181): Unconfirmed text disappears when text focus moves
+
+        The key was to change things around so that we don't push text from the DOM to the widget
+        unless the DOM has actually been changed. This prevents the code path that wipes out inline input
+        during the blur process.
+
+        * khtml/html/html_formimpl.cpp:
+        (DOM::HTMLInputElementImpl::HTMLInputElementImpl): Start m_valueMatchesRenderer as false.
+        (DOM::HTMLInputElementImpl::parseHTMLAttribute): Set m_valueMatchesRenderer to false when a
+        new value is set here.
+        (DOM::HTMLInputElementImpl::setValue): Set m_valueMatchesRenderer to false when a new value
+        is set here.
+        (DOM::HTMLInputElementImpl::setValueFromRenderer): Added. Sets m_value, sets m_valueMatchesRenderer
+        to true, and also sends out the input event. It's better to have this here than in the renderer code.
+        (DOM::HTMLTextAreaElementImpl::HTMLTextAreaElementImpl): Start m_valueIsValid as false (replaces
+        m_dirtyvalue) and m_valueMatchesRenderer as false.
+        (DOM::HTMLTextAreaElementImpl::updateValue): Added. Factored this out from the value function. Uses
+        the new booleans and keeps them up to date, specifically setting m_valueMatchesRenderer based on
+        where the value came from.
+        (DOM::HTMLTextAreaElementImpl::value): Updated to call updateValue to do most of the work.
+        (DOM::HTMLTextAreaElementImpl::setValue): Set both m_valueIsValid and m_valueMatchesRenderer.
+        (DOM::HTMLTextAreaElementImpl::setDefaultValue): Take parameter by reference.
+
+        * khtml/html/html_formimpl.h: Added setValueFromRenderer, valueMatchesRenderer, setValueMatchesRenderer,
+        and m_valueMatchesRenderer to input element. For textarea element, made some parameters pass DOMString
+        by reference, and added invalidateValue, updateValue, valueMatchesRenderer, and setValueMatchesRenderer.
+
+        * khtml/rendering/render_form.cpp:
+        (RenderLineEdit::updateFromElement): Don't re-get the value from the DOM if valueMatchesRenderer
+        is true.
+        (RenderLineEdit::slotTextChanged): Call setValueFromRenderer instead of manipulating the DOM
+        directly.
+        (RenderTextArea::detach): Call updateValue instead of calling value for its side effect.
+        (RenderTextArea::handleFocusOut): Ditto.
+        (RenderTextArea::updateFromElement): Call updateValue and then not re-get the value from the
+        DOM if valueMatchesRenderer is true.
+        (RenderTextArea::slotTextChanged): Call invalidateValue instead of directly setting m_dirtyvalue to true.
+
 2005-02-25  Chris Blumenberg  <cblu@apple.com>
 
        Fixed: <rdar://problem/4023566> Stickies: Crash in ReplacementFragment::insertFragmentForTestRendering on paste
index add19a74c66aa9744a94685d1db72a533c214052..fc7f0564d217c682ee71ccbb844d5dcd4b0e90f0 100644 (file)
@@ -1236,7 +1236,7 @@ RenderObject* HTMLFieldSetElementImpl::createRenderer(RenderArena* arena, Render
 // -------------------------------------------------------------------------
 
 HTMLInputElementImpl::HTMLInputElementImpl(DocumentPtr *doc, HTMLFormElementImpl *f)
-    : HTMLGenericFormElementImpl(doc, f), m_imageLoader(0)
+    : HTMLGenericFormElementImpl(doc, f), m_imageLoader(0), m_valueMatchesRenderer(false)
 {
     m_type = TEXT;
     m_maxLen = -1;
@@ -1532,6 +1532,7 @@ void HTMLInputElementImpl::parseHTMLAttribute(HTMLAttributeImpl *attr)
         // We only need to setChanged if the form is looking at the default value right now.
         if (m_value.isNull())
             setChanged();
+        m_valueMatchesRenderer = false;
         break;
     case ATTR_CHECKED:
         m_defaultChecked = !attr->isNull();
@@ -1936,6 +1937,16 @@ void HTMLInputElementImpl::setValue(const DOMString &value)
     } else {
         setAttribute(ATTR_VALUE, value);
     }
+    m_valueMatchesRenderer = false;
+}
+
+void HTMLInputElementImpl::setValueFromRenderer(const DOMString &value)
+{
+    m_value = value;
+    m_valueMatchesRenderer = true;
+    
+    // Fire the "input" DOM event.
+    dispatchHTMLEvent(EventImpl::INPUT_EVENT, true, false);
 }
 
 bool HTMLInputElementImpl::storesValueSeparateFromAttribute() const
@@ -2898,13 +2909,12 @@ HTMLSelectElementImpl *HTMLOptionElementImpl::getSelect() const
 // -------------------------------------------------------------------------
 
 HTMLTextAreaElementImpl::HTMLTextAreaElementImpl(DocumentPtr *doc, HTMLFormElementImpl *f)
-    : HTMLGenericFormElementImpl(doc, f)
+    : HTMLGenericFormElementImpl(doc, f), m_valueIsValid(false), m_valueMatchesRenderer(false)
 {
     // DTD requires rows & cols be specified, but we will provide reasonable defaults
     m_rows = 2;
     m_cols = 20;
     m_wrap = ta_Virtual;
-    m_dirtyvalue = true;
 }
 
 HTMLTextAreaElementImpl::~HTMLTextAreaElementImpl()
@@ -3017,25 +3027,31 @@ void HTMLTextAreaElementImpl::reset()
     setValue(defaultValue());
 }
 
-DOMString HTMLTextAreaElementImpl::value()
+void HTMLTextAreaElementImpl::updateValue()
 {
-    if ( m_dirtyvalue) {
-        if ( m_render )
+    if ( !m_valueIsValid ) {
+        if ( m_render ) {
             m_value = static_cast<RenderTextArea*>( m_render )->text();
-        else
+            m_valueMatchesRenderer = true;
+        } else {
             m_value = defaultValue().string();
-        m_dirtyvalue = false;
+            m_valueMatchesRenderer = false;
+        }
+        m_valueIsValid = true;
     }
+}
 
-    if ( m_value.isNull() ) return "";
-
-    return m_value;
+DOMString HTMLTextAreaElementImpl::value()
+{
+    updateValue();
+    return m_value.isNull() ? DOMString("") : m_value;
 }
 
-void HTMLTextAreaElementImpl::setValue(DOMString _value)
+void HTMLTextAreaElementImpl::setValue(const DOMString &value)
 {
-    m_value = _value.string();
-    m_dirtyvalue = false;
+    m_value = value.string();
+    m_valueIsValid = true;
+    m_valueMatchesRenderer = false;
     setChanged(true);
 }
 
@@ -3060,7 +3076,7 @@ DOMString HTMLTextAreaElementImpl::defaultValue()
     return val;
 }
 
-void HTMLTextAreaElementImpl::setDefaultValue(DOMString _defaultValue)
+void HTMLTextAreaElementImpl::setDefaultValue(const DOMString &defaultValue)
 {
     // there may be comments - remove all the text nodes and replace them with one
     QPtrList<NodeImpl> toRemove;
@@ -3073,8 +3089,8 @@ void HTMLTextAreaElementImpl::setDefaultValue(DOMString _defaultValue)
     for (; it.current(); ++it) {
         removeChild(it.current(), exceptioncode);
     }
-    insertBefore(getDocument()->createTextNode(_defaultValue),firstChild(), exceptioncode);
-    setValue(_defaultValue);
+    insertBefore(getDocument()->createTextNode(defaultValue),firstChild(), exceptioncode);
+    setValue(defaultValue);
 }
 
 void HTMLTextAreaElementImpl::blur()
index b189c8d498813a8cac2f36abe3f07a8af88227b3..d9377630e821385c69859f1f76541914b373e86f 100644 (file)
@@ -315,6 +315,10 @@ public:
     DOMString value() const;
     void setValue(const DOMString &);
 
+    void setValueFromRenderer(const DOMString &);
+    bool valueMatchesRenderer() const { return m_valueMatchesRenderer; }
+    void setValueMatchesRenderer() { m_valueMatchesRenderer = true; }
+
     void blur();
     void focus();
 
@@ -379,6 +383,7 @@ protected:
     bool m_haveType : 1;
     bool m_activeSubmit : 1;
     bool m_autocomplete : 1;
+    bool m_valueMatchesRenderer : 1;
 };
 
 // -------------------------------------------------------------------------
@@ -631,12 +636,18 @@ public:
     virtual bool appendFormData(FormDataList&, bool);
     virtual void reset();
     DOMString value();
-    void setValue(DOMString _value);
+    void setValue(const DOMString &value);
     DOMString defaultValue();
-    void setDefaultValue(DOMString _defaultValue);
+    void setDefaultValue(const DOMString &value);
     void blur();
     void focus();
 
+    void invalidateValue() { m_valueIsValid = false; }
+    void updateValue();
+
+    bool valueMatchesRenderer() const { return m_valueMatchesRenderer; }
+    void setValueMatchesRenderer() { m_valueMatchesRenderer = true; }
+
     virtual bool isEditable();
     
     virtual void accessKeyAction();
@@ -646,7 +657,8 @@ protected:
     int m_cols;
     WrapMethod m_wrap;
     QString m_value;
-    bool m_dirtyvalue;
+    bool m_valueIsValid;
+    bool m_valueMatchesRenderer;
 };
 
 // -------------------------------------------------------------------------
index 4fb3ef349c0272b22d624679dca201f0977eff63..134ede72f72e16dd5fe2c348bcecaf1c1a5dd11e 100644 (file)
@@ -660,43 +660,44 @@ void RenderLineEdit::setStyle(RenderStyle *s)
 
 void RenderLineEdit::updateFromElement()
 {
+    HTMLInputElementImpl *e = element();
     KLineEdit *w = widget();
     
-    int ml = element()->maxLength();
+    int ml = e->maxLength();
     if ( ml <= 0 || ml > 1024 )
         ml = 1024;
     if ( w->maxLength() != ml )
         w->setMaxLength( ml );
 
-    // Call w->text() before calling element()->value(), because in the case of inline
-    // input such as Hiragana, w->text() has a side effect of sending the notification
-    // that we use in slotTextChanged to update element()->m_value
-    QString widgetText = w->text();
-    QString newText = element()->value().string();
-    newText.replace(QChar('\\'), backslashAsCurrencySymbol());
-
-    if (newText != widgetText) {
-        w->blockSignals(true);
-        int pos = w->cursorPosition();
-
-        m_updating = true;
-        w->setText(newText);
-        m_updating = false;
-        
-        w->setEdited( false );
-
-        w->setCursorPosition(pos);
-        w->blockSignals(false);
+    if (!e->valueMatchesRenderer()) {
+        QString widgetText = w->text();
+        QString newText = e->value().string();
+        newText.replace(QChar('\\'), backslashAsCurrencySymbol());
+        if (widgetText != newText) {
+            w->blockSignals(true);
+            int pos = w->cursorPosition();
+
+            m_updating = true;
+            w->setText(newText);
+            m_updating = false;
+            
+            w->setEdited( false );
+
+            w->setCursorPosition(pos);
+            w->blockSignals(false);
+        }
+        e->setValueMatchesRenderer();
     }
-    w->setReadOnly(element()->readOnly());
+
+    w->setReadOnly(e->readOnly());
     
 #if APPLE_CHANGES
     // Handle updating the search attributes.
-    w->setPlaceholderString(element()->getAttribute(ATTR_PLACEHOLDER).string());
+    w->setPlaceholderString(e->getAttribute(ATTR_PLACEHOLDER).string());
     if (w->type() == QLineEdit::Search) {
-        w->setLiveSearch(!element()->getAttribute(ATTR_INCREMENTAL).isNull());
-        w->setAutoSaveName(element()->getAttribute(ATTR_AUTOSAVE).string());
-        w->setMaxResults(element()->maxResults());
+        w->setLiveSearch(!e->getAttribute(ATTR_INCREMENTAL).isNull());
+        w->setAutoSaveName(e->getAttribute(ATTR_AUTOSAVE).string());
+        w->setMaxResults(e->maxResults());
     }
 #endif
 
@@ -705,19 +706,15 @@ void RenderLineEdit::updateFromElement()
 
 void RenderLineEdit::slotTextChanged(const QString &string)
 {
-    // don't use setValue here!
-    if (m_updating) // Don't alter m_value if we are in the middle of initing the control, since
-        return;     // we may have gotten our initial value from the attribute.
+    if (m_updating) // Don't alter the value if we are in the middle of initing the control, since
+        return;     // we are getting the value from the DOM and it's not user input.
 
     // A null string value is used to indicate that the form control has not altered the original
     // default value.  That means that we should never use the null string value when the user
     // empties a textfield, but should always force an empty textfield to use the empty string.
     QString newText = string.isNull() ? "" : string;
     newText.replace(backslashAsCurrencySymbol(), QChar('\\'));
-    element()->m_value = newText;
-    
-    // Fire the "input" DOM event.
-    element()->dispatchHTMLEvent(EventImpl::INPUT_EVENT, true, false);
+    element()->setValueFromRenderer(newText);
 }
 
 void RenderLineEdit::select()
@@ -1501,14 +1498,14 @@ RenderTextArea::RenderTextArea(HTMLTextAreaElementImpl *element)
 
 void RenderTextArea::detach()
 {
-    element()->value(); // call this for the side effect of copying the value from render to DOM
+    element()->updateValue();
     RenderFormElement::detach();
 }
 
 void RenderTextArea::handleFocusOut()
 {
     if ( m_dirty && element() ) {
-        element()->value(); // call this for the side effect of copying the value from render to DOM
+        element()->updateValue();
         element()->onChange();
     }
     m_dirty = false;
@@ -1576,25 +1573,28 @@ void RenderTextArea::setStyle(RenderStyle *s)
 
 void RenderTextArea::updateFromElement()
 {
+    HTMLTextAreaElementImpl *e = element();
     QTextEdit* w = static_cast<QTextEdit*>(m_widget);
-    w->setReadOnly(element()->readOnly());
+
+    w->setReadOnly(e->readOnly());
 #if APPLE_CHANGES
-    w->setDisabled(element()->disabled());
+    w->setDisabled(e->disabled());
 #endif
-    
-    // Call text() before calling element()->value(), because in the case of inline
-    // input such as Hiragana, w->text() has a side effect of sending the notification
-    // that we use in slotTextChanged to update element()->m_value.
-    QString widgetText = text();
-    QString text = element()->value().string();
-    text.replace(QChar('\\'), backslashAsCurrencySymbol());
-    if (widgetText != text) {
-        w->blockSignals(true);
-        int line, col;
-        w->getCursorPosition( &line, &col );
-        w->setText(text);
-        w->setCursorPosition( line, col );
-        w->blockSignals(false);
+
+    e->updateValue();
+    if (!e->valueMatchesRenderer()) {
+        QString widgetText = text();
+        QString text = e->value().string();
+        text.replace(QChar('\\'), backslashAsCurrencySymbol());
+        if (widgetText != text) {
+            w->blockSignals(true);
+            int line, col;
+            w->getCursorPosition( &line, &col );
+            w->setText(text);
+            w->setCursorPosition( line, col );
+            w->blockSignals(false);
+        }
+        e->setValueMatchesRenderer();
         m_dirty = false;
     }
 
@@ -1637,7 +1637,7 @@ QString RenderTextArea::text()
 
 void RenderTextArea::slotTextChanged()
 {
-    element()->m_dirtyvalue = true;
+    element()->invalidateValue();
     m_dirty = true;
 }