[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    branches/KDE/4.0/kdelibs/khtml/rendering
From:       Maks Orlovich <maksim () kde ! org>
Date:       2008-01-17 17:33:09
Message-ID: 1200591189.876592.16725.nullmailer () svn ! kde ! org
[Download RAW message or body]

SVN commit 762684 by orlovich:

Do not emit onchange on synthetic toggling of radio buttons and checkboxes.
That's incompatible, and also led to #155973, crash on the beta BBC's page
location selector, as we have the following scenario:

1. JS sets checked.
2. We do updateFromElement, ask Qt to update the widget
3. The widget emits the change signal
4. The change signal handler does ref() [rc = 2]
5. The change signal handler does onchange(). The event running
causes a detach, which does a deref() [rc = 1]
6. The change signal handler does deref() [rc = 0], so the Render* gets destroyed
7. The common parts of updateFromElement, such as RenderWidget::updateFromElement, etc.,
run on a deleted RenderCheckBox/RadioButton, trying to access deleted
RenderStyle, etc. boom.

BUG: 155973

 M  +14 -2     render_form.cpp  
 M  +4 -0      render_form.h  


--- branches/KDE/4.0/kdelibs/khtml/rendering/render_form.cpp #762683:762684
@@ -242,6 +242,7 @@
     b->setChecked(element->checked());
 
     connect(b,SIGNAL(stateChanged(int)),this,SLOT(slotStateChanged(int)));
+    m_ignoreStateChanged = false;
 }
 
 
@@ -260,14 +261,19 @@
 
 void RenderCheckBox::updateFromElement()
 {
-    if (widget()->isChecked() != element()->checked())
+    if (widget()->isChecked() != element()->checked()) {
+        m_ignoreStateChanged = true; // We don't want an onchange here,
+                                     // or us getting yanked in a recalcStyle in the process, etc.
         widget()->setChecked(element()->checked());
+        m_ignoreStateChanged = false;
+    }
 
     RenderButton::updateFromElement();
 }
 
 void RenderCheckBox::slotStateChanged(int state)
 {
+    if (m_ignoreStateChanged) return;
     element()->setChecked(state == Qt::Checked);
 
     ref();
@@ -294,7 +300,7 @@
 
 RenderRadioButton::RenderRadioButton(HTMLInputElementImpl *element)
     : RenderButton(element)
-{
+{    
     RadioButtonWidget* b = new RadioButtonWidget(view()->widget());
     b->setMouseTracking(true);
     b->setAutoExclusive(false);
@@ -304,11 +310,14 @@
     b->setChecked(element->checked());
 
     connect(b,SIGNAL(toggled(bool)),this,SLOT(slotToggled(bool)));
+    m_ignoreToggled = false;
 }
 
 void RenderRadioButton::updateFromElement()
 {
+    m_ignoreToggled = true;
     widget()->setChecked(element()->checked());
+    m_ignoreToggled = false;
 
     RenderButton::updateFromElement();
 }
@@ -328,6 +337,9 @@
 
 void RenderRadioButton::slotToggled(bool activated)
 {
+    if (m_ignoreToggled)
+      return;
+
     if(activated) {
       ref();
       element()->onChange();
--- branches/KDE/4.0/kdelibs/khtml/rendering/render_form.h #762683:762684
@@ -153,6 +153,8 @@
 
 public Q_SLOTS:
     virtual void slotStateChanged(int state);
+private:
+    bool m_ignoreStateChanged;
 };
 
 // -------------------------------------------------------------------------
@@ -180,6 +182,8 @@
 
 public Q_SLOTS:
     virtual void slotToggled(bool);
+private:
+    bool m_ignoreToggled;
 };
 
 
[prev in list] [next in list] [prev in thread] [next in thread]