Reviewed by John.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Feb 2005 17:35:40 +0000 (17:35 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Feb 2005 17:35:40 +0000 (17:35 +0000)
        - 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@8666 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 cc1eec94522b80998b25aa1f475316d10ae28608..edfdf0ea274458f973e73a032e4b23150ae44515 100644 (file)
@@ -1,3 +1,45 @@
+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-22  Richard Williamson   <rjw@apple.com>
 
        Fixed <rdar://problem/3937203> when a div adds a scrollbar (overflow:auto) we do not get regions
index 831bd4109cb0a52abdd1a72c97f311108f1938a9..a02c95f5672fc3fc85c0192b0325620724b214b5 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();
@@ -1935,6 +1936,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
@@ -2897,13 +2908,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()
@@ -3016,25 +3026,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);
 }
 
@@ -3059,7 +3075,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;
@@ -3072,8 +3088,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 ad3fc6629560af3463f2bc2a1d0006794c0d6b3f..1a287224acf224e52ad9809ddbc2baf96e37d2d0 100644 (file)
@@ -659,43 +659,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
 
@@ -704,19 +705,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()
@@ -1500,14 +1497,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;
@@ -1575,25 +1572,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;
     }
 
@@ -1636,7 +1636,7 @@ QString RenderTextArea::text()
 
 void RenderTextArea::slotTextChanged()
 {
-    element()->m_dirtyvalue = true;
+    element()->invalidateValue();
     m_dirty = true;
 }