<!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>[243684] trunk</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/243684">243684</a></dd>
<dt>Author</dt> <dd>wenson_hsieh@apple.com</dd>
<dt>Date</dt> <dd>2019-03-31 13:01:44 -0700 (Sun, 31 Mar 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>[iOS] Crash when changing inputmode for certain types of focusable elements
https://bugs.webkit.org/show_bug.cgi?id=196431
<rdar://problem/49454962>

Reviewed by Tim Horton.

Source/WebKit:

The crash is happening because WebPage::focusedElementDidChangeInputMode assumes that the document's focused
element must be the same as m_focusedElement in WebPage. However, this is not the case, since m_focusedElement
is only set for certain types of elements that require user input (e.g. text fields, editable content, select
menus, etc.). The function then attempts to dereference m_focusedElement, which may be null if the document's
focused element doesn't fall into one of the aforementioned categories.

To fix this, bail if the element that is changing inputmode is not equal to the WebPage's current focused
element. See below for more details.

Test: fast/forms/change-inputmode-crash.html

* WebProcess/WebPage/WebPage.cpp:
(WebKit::isTextFormControlOrEditableContent):

Clean up some existing logic by introducing a helper method for determining whether an element should
propagate inputmode attribute changes to the UI process. Also, check the element type using type traits instead
of checking against the tag name.

(WebKit::WebPage::elementDidFocus):
(WebKit::WebPage::focusedElementDidChangeInputMode):

LayoutTests:

Add a layout test that exercises the edge case; see WebKit ChangeLogs for more details.

* fast/forms/change-inputmode-crash-expected.txt: Added.
* fast/forms/change-inputmode-crash.html: Added.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitChangeLog">trunk/Source/WebKit/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitWebProcessWebPageWebPagecpp">trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastformschangeinputmodecrashexpectedtxt">trunk/LayoutTests/fast/forms/change-inputmode-crash-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastformschangeinputmodecrashhtml">trunk/LayoutTests/fast/forms/change-inputmode-crash.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (243683 => 243684)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2019-03-31 19:29:56 UTC (rev 243683)
+++ trunk/LayoutTests/ChangeLog 2019-03-31 20:01:44 UTC (rev 243684)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2019-03-31  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Crash when changing inputmode for certain types of focusable elements
+        https://bugs.webkit.org/show_bug.cgi?id=196431
+        <rdar://problem/49454962>
+
+        Reviewed by Tim Horton.
+
+        Add a layout test that exercises the edge case; see WebKit ChangeLogs for more details.
+
+        * fast/forms/change-inputmode-crash-expected.txt: Added.
+        * fast/forms/change-inputmode-crash.html: Added.
+
</ins><span class="cx"> 2019-03-29  Dean Jackson  <dino@apple.com>
</span><span class="cx"> 
</span><span class="cx">         gl.readPixels with type gl.FLOAT does not work
</span></span></pre></div>
<a id="trunkLayoutTestsfastformschangeinputmodecrashexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/forms/change-inputmode-crash-expected.txt (0 => 243684)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/forms/change-inputmode-crash-expected.txt                         (rev 0)
+++ trunk/LayoutTests/fast/forms/change-inputmode-crash-expected.txt    2019-03-31 20:01:44 UTC (rev 243684)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+This test verifies that changing the inputmode attribute after changing focus does not cause a crash. To manually run the test, load the page and verify that a crash does not occur.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.activeElement is target
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastformschangeinputmodecrashhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/forms/change-inputmode-crash.html (0 => 243684)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/forms/change-inputmode-crash.html                         (rev 0)
+++ trunk/LayoutTests/fast/forms/change-inputmode-crash.html    2019-03-31 20:01:44 UTC (rev 243684)
</span><span class="lines">@@ -0,0 +1,17 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+<span id="target" tabindex="0" style="width: 100px; height: 100px;"></span>
+<script>
+description("This test verifies that changing the inputmode attribute after changing focus does not cause a crash. To manually run the test, load the page and verify that a crash does not occur.");
+
+target = document.getElementById("target");
+target.focus();
+target.setAttribute("inputmode", "url");
+shouldBe("document.activeElement", "target");
+</script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/ChangeLog (243683 => 243684)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/ChangeLog    2019-03-31 19:29:56 UTC (rev 243683)
+++ trunk/Source/WebKit/ChangeLog       2019-03-31 20:01:44 UTC (rev 243684)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2019-03-31  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Crash when changing inputmode for certain types of focusable elements
+        https://bugs.webkit.org/show_bug.cgi?id=196431
+        <rdar://problem/49454962>
+
+        Reviewed by Tim Horton.
+
+        The crash is happening because WebPage::focusedElementDidChangeInputMode assumes that the document's focused
+        element must be the same as m_focusedElement in WebPage. However, this is not the case, since m_focusedElement
+        is only set for certain types of elements that require user input (e.g. text fields, editable content, select
+        menus, etc.). The function then attempts to dereference m_focusedElement, which may be null if the document's
+        focused element doesn't fall into one of the aforementioned categories.
+
+        To fix this, bail if the element that is changing inputmode is not equal to the WebPage's current focused
+        element. See below for more details.
+
+        Test: fast/forms/change-inputmode-crash.html
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::isTextFormControlOrEditableContent):
+
+        Clean up some existing logic by introducing a helper method for determining whether an element should
+        propagate inputmode attribute changes to the UI process. Also, check the element type using type traits instead
+        of checking against the tag name.
+
+        (WebKit::WebPage::elementDidFocus):
+        (WebKit::WebPage::focusedElementDidChangeInputMode):
+
</ins><span class="cx"> 2019-03-31  Sam Weinig  <weinig@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Remove more i386 specific configurations
</span></span></pre></div>
<a id="trunkSourceWebKitWebProcessWebPageWebPagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (243683 => 243684)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp       2019-03-31 19:29:56 UTC (rev 243683)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp  2019-03-31 20:01:44 UTC (rev 243684)
</span><span class="lines">@@ -173,7 +173,9 @@
</span><span class="cx"> #include <WebCore/HTMLOListElement.h>
</span><span class="cx"> #include <WebCore/HTMLPlugInElement.h>
</span><span class="cx"> #include <WebCore/HTMLPlugInImageElement.h>
</span><ins>+#include <WebCore/HTMLSelectElement.h>
</ins><span class="cx"> #include <WebCore/HTMLTextAreaElement.h>
</span><ins>+#include <WebCore/HTMLTextFormControlElement.h>
</ins><span class="cx"> #include <WebCore/HTMLUListElement.h>
</span><span class="cx"> #include <WebCore/HistoryController.h>
</span><span class="cx"> #include <WebCore/HistoryItem.h>
</span><span class="lines">@@ -5345,6 +5347,11 @@
</span><span class="cx">     return true;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+static bool isTextFormControlOrEditableContent(const WebCore::Element& element)
+{
+    return is<HTMLTextFormControlElement>(element) || element.hasEditableStyle();
+}
+
</ins><span class="cx"> void WebPage::elementDidFocus(WebCore::Element& element)
</span><span class="cx"> {
</span><span class="cx">     if (!shouldDispatchUpdateAfterFocusingElement(element)) {
</span><span class="lines">@@ -5353,7 +5360,7 @@
</span><span class="cx">         return;
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    if (element.hasTagName(WebCore::HTMLNames::selectTag) || element.hasTagName(WebCore::HTMLNames::inputTag) || element.hasTagName(WebCore::HTMLNames::textareaTag) || element.hasEditableStyle()) {
</del><ins>+    if (is<HTMLSelectElement>(element) || isTextFormControlOrEditableContent(element)) {
</ins><span class="cx">         m_focusedElement = &element;
</span><span class="cx"> 
</span><span class="cx"> #if PLATFORM(IOS_FAMILY)
</span><span class="lines">@@ -5399,17 +5406,18 @@
</span><span class="cx"> 
</span><span class="cx"> void WebPage::focusedElementDidChangeInputMode(WebCore::Element& element, WebCore::InputMode mode)
</span><span class="cx"> {
</span><ins>+    if (m_focusedElement != &element)
+        return;
+
</ins><span class="cx"> #if PLATFORM(IOS_FAMILY)
</span><del>-    ASSERT(m_focusedElement == &element);
</del><span class="cx">     ASSERT(is<HTMLElement>(element));
</span><span class="cx">     ASSERT(downcast<HTMLElement>(element).canonicalInputMode() == mode);
</span><span class="cx"> 
</span><del>-    if (!is<HTMLTextAreaElement>(*m_focusedElement) && !is<HTMLInputElement>(*m_focusedElement) && !m_focusedElement->hasEditableStyle())
</del><ins>+    if (!isTextFormControlOrEditableContent(element))
</ins><span class="cx">         return;
</span><span class="cx"> 
</span><span class="cx">     send(Messages::WebPageProxy::FocusedElementDidChangeInputMode(mode));
</span><span class="cx"> #else
</span><del>-    UNUSED_PARAM(element);
</del><span class="cx">     UNUSED_PARAM(mode);
</span><span class="cx"> #endif
</span><span class="cx"> }
</span></span></pre>
</div>
</div>

</body>
</html>