Reviewed by Darin.
authorvicki <vicki@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Feb 2005 18:36:44 +0000 (18:36 +0000)
committervicki <vicki@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Feb 2005 18:36:44 +0000 (18:36 +0000)
- back out this change, since it causes a 3.5% 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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@8695 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 5af4ec6e4372d92a2a4462103a44fc45bcd73d6e..b6a4407dd61a34c720cf679edd055c170b07fc75 100644 (file)
@@ -1,3 +1,51 @@
+2005-02-25  Vicki Murley <vicki@apple.com>
+
+        Reviewed by Darin.
+       - back out this change, since it causes a 3.5% 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  Darin Adler  <darin@apple.com>
 
         Reviewed by Chris.
index fc7f0564d217c682ee71ccbb844d5dcd4b0e90f0..add19a74c66aa9744a94685d1db72a533c214052 100644 (file)
@@ -1236,7 +1236,7 @@ RenderObject* HTMLFieldSetElementImpl::createRenderer(RenderArena* arena, Render
 // -------------------------------------------------------------------------
 
 HTMLInputElementImpl::HTMLInputElementImpl(DocumentPtr *doc, HTMLFormElementImpl *f)
-    : HTMLGenericFormElementImpl(doc, f), m_imageLoader(0), m_valueMatchesRenderer(false)
+    : HTMLGenericFormElementImpl(doc, f), m_imageLoader(0)
 {
     m_type = TEXT;
     m_maxLen = -1;
@@ -1532,7 +1532,6 @@ 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();
@@ -1937,16 +1936,6 @@ 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
@@ -2909,12 +2898,13 @@ HTMLSelectElementImpl *HTMLOptionElementImpl::getSelect() const
 // -------------------------------------------------------------------------
 
 HTMLTextAreaElementImpl::HTMLTextAreaElementImpl(DocumentPtr *doc, HTMLFormElementImpl *f)
-    : HTMLGenericFormElementImpl(doc, f), m_valueIsValid(false), m_valueMatchesRenderer(false)
+    : HTMLGenericFormElementImpl(doc, f)
 {
     // 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()
@@ -3027,31 +3017,25 @@ void HTMLTextAreaElementImpl::reset()
     setValue(defaultValue());
 }
 
-void HTMLTextAreaElementImpl::updateValue()
+DOMString HTMLTextAreaElementImpl::value()
 {
-    if ( !m_valueIsValid ) {
-        if ( m_render ) {
+    if ( m_dirtyvalue) {
+        if ( m_render )
             m_value = static_cast<RenderTextArea*>( m_render )->text();
-            m_valueMatchesRenderer = true;
-        } else {
+        else
             m_value = defaultValue().string();
-            m_valueMatchesRenderer = false;
-        }
-        m_valueIsValid = true;
+        m_dirtyvalue = false;
     }
-}
 
-DOMString HTMLTextAreaElementImpl::value()
-{
-    updateValue();
-    return m_value.isNull() ? DOMString("") : m_value;
+    if ( m_value.isNull() ) return "";
+
+    return m_value;
 }
 
-void HTMLTextAreaElementImpl::setValue(const DOMString &value)
+void HTMLTextAreaElementImpl::setValue(DOMString _value)
 {
-    m_value = value.string();
-    m_valueIsValid = true;
-    m_valueMatchesRenderer = false;
+    m_value = _value.string();
+    m_dirtyvalue = false;
     setChanged(true);
 }
 
@@ -3076,7 +3060,7 @@ DOMString HTMLTextAreaElementImpl::defaultValue()
     return val;
 }
 
-void HTMLTextAreaElementImpl::setDefaultValue(const DOMString &defaultValue)
+void HTMLTextAreaElementImpl::setDefaultValue(DOMString _defaultValue)
 {
     // there may be comments - remove all the text nodes and replace them with one
     QPtrList<NodeImpl> toRemove;
@@ -3089,8 +3073,8 @@ void HTMLTextAreaElementImpl::setDefaultValue(const 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 d9377630e821385c69859f1f76541914b373e86f..b189c8d498813a8cac2f36abe3f07a8af88227b3 100644 (file)
@@ -315,10 +315,6 @@ 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();
 
@@ -383,7 +379,6 @@ protected:
     bool m_haveType : 1;
     bool m_activeSubmit : 1;
     bool m_autocomplete : 1;
-    bool m_valueMatchesRenderer : 1;
 };
 
 // -------------------------------------------------------------------------
@@ -636,18 +631,12 @@ public:
     virtual bool appendFormData(FormDataList&, bool);
     virtual void reset();
     DOMString value();
-    void setValue(const DOMString &value);
+    void setValue(DOMString _value);
     DOMString defaultValue();
-    void setDefaultValue(const DOMString &value);
+    void setDefaultValue(DOMString _defaultValue);
     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();
@@ -657,8 +646,7 @@ protected:
     int m_cols;
     WrapMethod m_wrap;
     QString m_value;
-    bool m_valueIsValid;
-    bool m_valueMatchesRenderer;
+    bool m_dirtyvalue;
 };
 
 // -------------------------------------------------------------------------
index 134ede72f72e16dd5fe2c348bcecaf1c1a5dd11e..4fb3ef349c0272b22d624679dca201f0977eff63 100644 (file)
@@ -660,44 +660,43 @@ void RenderLineEdit::setStyle(RenderStyle *s)
 
 void RenderLineEdit::updateFromElement()
 {
-    HTMLInputElementImpl *e = element();
     KLineEdit *w = widget();
     
-    int ml = e->maxLength();
+    int ml = element()->maxLength();
     if ( ml <= 0 || ml > 1024 )
         ml = 1024;
     if ( w->maxLength() != ml )
         w->setMaxLength( ml );
 
-    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();
+    // 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);
     }
-
-    w->setReadOnly(e->readOnly());
+    w->setReadOnly(element()->readOnly());
     
 #if APPLE_CHANGES
     // Handle updating the search attributes.
-    w->setPlaceholderString(e->getAttribute(ATTR_PLACEHOLDER).string());
+    w->setPlaceholderString(element()->getAttribute(ATTR_PLACEHOLDER).string());
     if (w->type() == QLineEdit::Search) {
-        w->setLiveSearch(!e->getAttribute(ATTR_INCREMENTAL).isNull());
-        w->setAutoSaveName(e->getAttribute(ATTR_AUTOSAVE).string());
-        w->setMaxResults(e->maxResults());
+        w->setLiveSearch(!element()->getAttribute(ATTR_INCREMENTAL).isNull());
+        w->setAutoSaveName(element()->getAttribute(ATTR_AUTOSAVE).string());
+        w->setMaxResults(element()->maxResults());
     }
 #endif
 
@@ -706,15 +705,19 @@ void RenderLineEdit::updateFromElement()
 
 void RenderLineEdit::slotTextChanged(const QString &string)
 {
-    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.
+    // 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.
 
     // 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()->setValueFromRenderer(newText);
+    element()->m_value = newText;
+    
+    // Fire the "input" DOM event.
+    element()->dispatchHTMLEvent(EventImpl::INPUT_EVENT, true, false);
 }
 
 void RenderLineEdit::select()
@@ -1498,14 +1501,14 @@ RenderTextArea::RenderTextArea(HTMLTextAreaElementImpl *element)
 
 void RenderTextArea::detach()
 {
-    element()->updateValue();
+    element()->value(); // call this for the side effect of copying the value from render to DOM
     RenderFormElement::detach();
 }
 
 void RenderTextArea::handleFocusOut()
 {
     if ( m_dirty && element() ) {
-        element()->updateValue();
+        element()->value(); // call this for the side effect of copying the value from render to DOM
         element()->onChange();
     }
     m_dirty = false;
@@ -1573,28 +1576,25 @@ void RenderTextArea::setStyle(RenderStyle *s)
 
 void RenderTextArea::updateFromElement()
 {
-    HTMLTextAreaElementImpl *e = element();
     QTextEdit* w = static_cast<QTextEdit*>(m_widget);
-
-    w->setReadOnly(e->readOnly());
+    w->setReadOnly(element()->readOnly());
 #if APPLE_CHANGES
-    w->setDisabled(e->disabled());
+    w->setDisabled(element()->disabled());
 #endif
-
-    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();
+    
+    // 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);
         m_dirty = false;
     }
 
@@ -1637,7 +1637,7 @@ QString RenderTextArea::text()
 
 void RenderTextArea::slotTextChanged()
 {
-    element()->invalidateValue();
+    element()->m_dirtyvalue = true;
     m_dirty = true;
 }