<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[47048] trunk/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/47048">47048</a></dd>
<dt>Author</dt> <dd>zimmermann@webkit.org</dd>
<dt>Date</dt> <dd>2009-08-11 12:29:18 -0700 (Tue, 11 Aug 2009)</dd>
</dl>

<h3>Log Message</h3>
<pre>2009-08-11  Nikolas Zimmermann  &lt;nikolas.zimmermann@torchmobile.com&gt;

        Reviewed by George Staikos.

        [WML] WMLPageState is not allowed to store the active card, it needs to be done per document
        https://bugs.webkit.org/show_bug.cgi?id=28180

        Don't store the active WMLCardElement in WMLPageState, but in WMLDocument.
        Otherwhise this may lead to crashes related to intrinsic event exeuction.

        Unfortunately select elements aren't testable by the layout tests, so adding
        a new manual test reproducing the crash.

        * manual-tests/wml/select-onpick-event-crash.wml: Added.
        * wml/WMLCardElement.cpp:
        (WebCore::WMLCardElement::determineActiveCard):
        * wml/WMLDoElement.cpp:
        (WebCore::WMLDoElement::defaultEventHandler):
        * wml/WMLDocument.cpp:
        (WebCore::WMLDocument::finishedParsing):
        * wml/WMLDocument.h:
        (WebCore::WMLDocument::activeCard):
        * wml/WMLGoElement.cpp:
        (WebCore::WMLGoElement::executeTask):
        * wml/WMLPageState.cpp:
        (WebCore::WMLPageState::WMLPageState):
        * wml/WMLPageState.h:
        * wml/WMLPrevElement.cpp:
        (WebCore::WMLPrevElement::executeTask):
        * wml/WMLRefreshElement.cpp:
        (WebCore::WMLRefreshElement::executeTask):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkWebCoreChangeLog">trunk/WebCore/ChangeLog</a></li>
<li><a href="#trunkWebCorewmlWMLCardElementcpp">trunk/WebCore/wml/WMLCardElement.cpp</a></li>
<li><a href="#trunkWebCorewmlWMLDoElementcpp">trunk/WebCore/wml/WMLDoElement.cpp</a></li>
<li><a href="#trunkWebCorewmlWMLDocumentcpp">trunk/WebCore/wml/WMLDocument.cpp</a></li>
<li><a href="#trunkWebCorewmlWMLDocumenth">trunk/WebCore/wml/WMLDocument.h</a></li>
<li><a href="#trunkWebCorewmlWMLGoElementcpp">trunk/WebCore/wml/WMLGoElement.cpp</a></li>
<li><a href="#trunkWebCorewmlWMLPageStatecpp">trunk/WebCore/wml/WMLPageState.cpp</a></li>
<li><a href="#trunkWebCorewmlWMLPageStateh">trunk/WebCore/wml/WMLPageState.h</a></li>
<li><a href="#trunkWebCorewmlWMLPrevElementcpp">trunk/WebCore/wml/WMLPrevElement.cpp</a></li>
<li><a href="#trunkWebCorewmlWMLRefreshElementcpp">trunk/WebCore/wml/WMLRefreshElement.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkWebCoremanualtestswmlselectonpickeventcrashwml">trunk/WebCore/manual-tests/wml/select-onpick-event-crash.wml</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/ChangeLog (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/ChangeLog        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/ChangeLog        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2009-08-11  Nikolas Zimmermann  &lt;nikolas.zimmermann@torchmobile.com&gt;
+
+        Reviewed by George Staikos.
+
+        [WML] WMLPageState is not allowed to store the active card, it needs to be done per document
+        https://bugs.webkit.org/show_bug.cgi?id=28180
+
+        Don't store the active WMLCardElement in WMLPageState, but in WMLDocument.
+        Otherwhise this may lead to crashes related to intrinsic event exeuction.
+
+        Unfortunately select elements aren't testable by the layout tests, so adding
+        a new manual test reproducing the crash.
+
+        * manual-tests/wml/select-onpick-event-crash.wml: Added.
+        * wml/WMLCardElement.cpp:
+        (WebCore::WMLCardElement::determineActiveCard):
+        * wml/WMLDoElement.cpp:
+        (WebCore::WMLDoElement::defaultEventHandler):
+        * wml/WMLDocument.cpp:
+        (WebCore::WMLDocument::finishedParsing):
+        * wml/WMLDocument.h:
+        (WebCore::WMLDocument::activeCard):
+        * wml/WMLGoElement.cpp:
+        (WebCore::WMLGoElement::executeTask):
+        * wml/WMLPageState.cpp:
+        (WebCore::WMLPageState::WMLPageState):
+        * wml/WMLPageState.h:
+        * wml/WMLPrevElement.cpp:
+        (WebCore::WMLPrevElement::executeTask):
+        * wml/WMLRefreshElement.cpp:
+        (WebCore::WMLRefreshElement::executeTask):
+
</ins><span class="cx"> 2009-08-07  Peter Kasting  &lt;pkasting@google.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Reviewed by Eric Seidel.
</span></span></pre></div>
<a id="trunkWebCoremanualtestswmlselectonpickeventcrashwml"></a>
<div class="addfile"><h4>Added: trunk/WebCore/manual-tests/wml/select-onpick-event-crash.wml (0 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/manual-tests/wml/select-onpick-event-crash.wml                                (rev 0)
+++ trunk/WebCore/manual-tests/wml/select-onpick-event-crash.wml        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -0,0 +1,12 @@
</span><ins>+&lt;?xml version=&quot;1.0&quot;?&gt;
+&lt;!DOCTYPE wml PUBLIC &quot;-//WAPFORUM//DTD WML 1.1//EN&quot; &quot;http://www.wapforum.org/DTD/wml_1.1.xml&quot;&gt;
+&lt;wml&gt;
+    &lt;card&gt;
+        &lt;select name=&quot;result&quot; dmultiple=&quot;true&quot;&gt;
+            &lt;option onpick=&quot;#card2&quot; value=&quot;foo&quot;&gt;Foo&lt;/option&gt;
+            &lt;option onpick=&quot;#card2&quot; value=&quot;bar&quot;&gt;Bar&lt;/option&gt;
+        &lt;/select&gt;
+    &lt;/card&gt;
+
+    &lt;card id=&quot;card2&quot;&gt;Test pass, if it didn't crash.&lt;/card&gt;
+&lt;/wml&gt;
</ins></span></pre></div>
<a id="trunkWebCorewmlWMLCardElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/wml/WMLCardElement.cpp (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/wml/WMLCardElement.cpp        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/wml/WMLCardElement.cpp        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -339,8 +339,6 @@
</span><span class="cx">     // Update the document title
</span><span class="cx">     doc-&gt;setTitle(activeCard-&gt;title());
</span><span class="cx"> 
</span><del>-    // Set the active activeCard in the WMLPageState object
-    pageState-&gt;setActiveCard(activeCard);
</del><span class="cx">     return activeCard;
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkWebCorewmlWMLDoElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/wml/WMLDoElement.cpp (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/wml/WMLDoElement.cpp        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/wml/WMLDoElement.cpp        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -72,12 +72,15 @@
</span><span class="cx">         if (m_task)
</span><span class="cx">             m_task-&gt;executeTask(event);
</span><span class="cx">     } else if (m_type == &quot;prev&quot;) {
</span><del>-        WMLPageState* pageState = wmlPageStateForDocument(document());
</del><ins>+        ASSERT(document()-&gt;isWMLDocument());
+        WMLDocument* document = static_cast&lt;WMLDocument*&gt;(this-&gt;document());
+
+        WMLPageState* pageState = wmlPageStateForDocument(document);
</ins><span class="cx">         if (!pageState)
</span><span class="cx">             return;
</span><del>-
</del><ins>+    
</ins><span class="cx">         // Stop the timer of the current card if it is active
</span><del>-        if (WMLCardElement* card = pageState-&gt;activeCard()) {
</del><ins>+        if (WMLCardElement* card = document-&gt;activeCard()) {
</ins><span class="cx">             if (WMLTimerElement* eventTimer = card-&gt;eventTimer())
</span><span class="cx">                 eventTimer-&gt;stop();
</span><span class="cx">         }
</span></span></pre></div>
<a id="trunkWebCorewmlWMLDocumentcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/wml/WMLDocument.cpp (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/wml/WMLDocument.cpp        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/wml/WMLDocument.cpp        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -79,10 +79,8 @@
</span><span class="cx">         return;
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    if (m_activeCard) {
</del><ins>+    if (m_activeCard)
</ins><span class="cx">         m_activeCard-&gt;handleIntrinsicEventIfNeeded();
</span><del>-        m_activeCard = 0;
-    }
</del><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> bool WMLDocument::initialize(bool aboutToFinishParsing)
</span></span></pre></div>
<a id="trunkWebCorewmlWMLDocumenth"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/wml/WMLDocument.h (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/wml/WMLDocument.h        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/wml/WMLDocument.h        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -44,6 +44,8 @@
</span><span class="cx"> 
</span><span class="cx">     bool initialize(bool aboutToFinishParsing = false);
</span><span class="cx"> 
</span><ins>+    WMLCardElement* activeCard() const { return m_activeCard; }
+
</ins><span class="cx"> private:
</span><span class="cx">     WMLDocument(Frame*);
</span><span class="cx">     WMLCardElement* m_activeCard;
</span></span></pre></div>
<a id="trunkWebCorewmlWMLGoElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/wml/WMLGoElement.cpp (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/wml/WMLGoElement.cpp        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/wml/WMLGoElement.cpp        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -75,16 +75,18 @@
</span><span class="cx"> 
</span><span class="cx"> void WMLGoElement::executeTask(Event*)
</span><span class="cx"> {
</span><del>-    Document* doc = document();
-    WMLPageState* pageState = wmlPageStateForDocument(doc);
</del><ins>+    ASSERT(document()-&gt;isWMLDocument());
+    WMLDocument* document = static_cast&lt;WMLDocument*&gt;(this-&gt;document());
+
+    WMLPageState* pageState = wmlPageStateForDocument(document);
</ins><span class="cx">     if (!pageState)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    WMLCardElement* card = pageState-&gt;activeCard();
</del><ins>+    WMLCardElement* card = document-&gt;activeCard();
</ins><span class="cx">     if (!card)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    Frame* frame = doc-&gt;frame();
</del><ins>+    Frame* frame = document-&gt;frame();
</ins><span class="cx">     if (!frame)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><span class="lines">@@ -97,7 +99,7 @@
</span><span class="cx">         return;
</span><span class="cx"> 
</span><span class="cx">     // Substitute variables within target url attribute value
</span><del>-    KURL url = doc-&gt;completeURL(substituteVariableReferences(href, doc, WMLVariableEscapingEscape));
</del><ins>+    KURL url = document-&gt;completeURL(substituteVariableReferences(href, document, WMLVariableEscapingEscape));
</ins><span class="cx">     if (url.isEmpty())
</span><span class="cx">         return;
</span><span class="cx"> 
</span><span class="lines">@@ -108,9 +110,9 @@
</span><span class="cx">         eventTimer-&gt;stop();
</span><span class="cx"> 
</span><span class="cx">     // FIXME: 'newcontext' handling not implemented for external cards
</span><del>-    bool inSameDeck = doc-&gt;url().path() == url.path();
</del><ins>+    bool inSameDeck = document-&gt;url().path() == url.path();
</ins><span class="cx">     if (inSameDeck &amp;&amp; url.hasFragmentIdentifier()) {
</span><del>-        if (WMLCardElement* card = WMLCardElement::findNamedCardInDocument(doc, url.fragmentIdentifier())) {
</del><ins>+        if (WMLCardElement* card = WMLCardElement::findNamedCardInDocument(document, url.fragmentIdentifier())) {
</ins><span class="cx">             if (card-&gt;isNewContext())
</span><span class="cx">                 pageState-&gt;reset();
</span><span class="cx">         }
</span></span></pre></div>
<a id="trunkWebCorewmlWMLPageStatecpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/wml/WMLPageState.cpp (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/wml/WMLPageState.cpp        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/wml/WMLPageState.cpp        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -35,7 +35,6 @@
</span><span class="cx"> 
</span><span class="cx"> WMLPageState::WMLPageState(Page* page)
</span><span class="cx">     : m_page(page)
</span><del>-    , m_activeCard(0)
</del><span class="cx">     , m_hasAccessControlData(false)
</span><span class="cx"> {
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkWebCorewmlWMLPageStateh"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/wml/WMLPageState.h (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/wml/WMLPageState.h        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/wml/WMLPageState.h        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -54,9 +54,6 @@
</span><span class="cx"> 
</span><span class="cx">     Page* page() const { return m_page; }
</span><span class="cx"> 
</span><del>-    WMLCardElement* activeCard() const { return m_activeCard; }
-    void setActiveCard(WMLCardElement* card) { m_activeCard = card; }
-
</del><span class="cx">     // Deck access control
</span><span class="cx">     bool processAccessControlData(const String&amp; dmain, const String&amp; path);
</span><span class="cx">     void resetAccessControlData();
</span><span class="lines">@@ -70,7 +67,6 @@
</span><span class="cx"> private:
</span><span class="cx">     Page* m_page;
</span><span class="cx">     WMLVariableMap m_variables;
</span><del>-    WMLCardElement* m_activeCard;
</del><span class="cx">     String m_accessDomain;
</span><span class="cx">     String m_accessPath;
</span><span class="cx">     bool m_hasAccessControlData;
</span></span></pre></div>
<a id="trunkWebCorewmlWMLPrevElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/wml/WMLPrevElement.cpp (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/wml/WMLPrevElement.cpp        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/wml/WMLPrevElement.cpp        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -42,11 +42,14 @@
</span><span class="cx"> 
</span><span class="cx"> void WMLPrevElement::executeTask(Event*)
</span><span class="cx"> {
</span><del>-    WMLPageState* pageState = wmlPageStateForDocument(document());
</del><ins>+    ASSERT(document()-&gt;isWMLDocument());
+    WMLDocument* document = static_cast&lt;WMLDocument*&gt;(this-&gt;document());
+
+    WMLPageState* pageState = wmlPageStateForDocument(document);
</ins><span class="cx">     if (!pageState)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    WMLCardElement* card = pageState-&gt;activeCard();
</del><ins>+    WMLCardElement* card = document-&gt;activeCard();
</ins><span class="cx">     if (!card)
</span><span class="cx">         return;
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkWebCorewmlWMLRefreshElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/wml/WMLRefreshElement.cpp (47047 => 47048)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/wml/WMLRefreshElement.cpp        2009-08-11 19:28:48 UTC (rev 47047)
+++ trunk/WebCore/wml/WMLRefreshElement.cpp        2009-08-11 19:29:18 UTC (rev 47048)
</span><span class="lines">@@ -43,11 +43,14 @@
</span><span class="cx"> 
</span><span class="cx"> void WMLRefreshElement::executeTask(Event*)
</span><span class="cx"> {
</span><del>-    WMLPageState* pageState = wmlPageStateForDocument(document());
</del><ins>+    ASSERT(document()-&gt;isWMLDocument());
+    WMLDocument* document = static_cast&lt;WMLDocument*&gt;(this-&gt;document());
+
+    WMLPageState* pageState = wmlPageStateForDocument(document);
</ins><span class="cx">     if (!pageState)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    WMLCardElement* card = pageState-&gt;activeCard();
</del><ins>+    WMLCardElement* card = document-&gt;activeCard();
</ins><span class="cx">     if (!card)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><span class="lines">@@ -62,7 +65,7 @@
</span><span class="cx">     storeVariableState(pageState);
</span><span class="cx"> 
</span><span class="cx">     // Redisplay curremt card with current variable state
</span><del>-    if (Frame* frame = document()-&gt;frame()) {
</del><ins>+    if (Frame* frame = document-&gt;frame()) {
</ins><span class="cx">         if (FrameLoader* loader = frame-&gt;loader())
</span><span class="cx">             loader-&gt;reload();
</span><span class="cx">     }
</span></span></pre>
</div>
</div>

</body>
</html>