🐞WebKitへのバグレポート

Safari 12では、日本語入力中でもTabキーによるフォーカス移動が動作する。

過去のバージョンやChromeなどとは動作が異なるためリグレッションバグと考え、WebKitにバグレポート(#188370)を行なった。 修正コミットがtrunkにされたので、いずれSafariでも修正されるはず。(2018-8-17追記: Version 12.0 (14606.1.36.1.3)で反映された)

💥遭遇

AquaSKKでは入力中にTabキーを押すと入力内容が補完される。 これを使ってTweetしようとした際に、フォーカスが移動してしまって困った。

問題箇所の切り分けのために、いろいろな環境で動作を確認した。その結果、以下のことが分かった。

• 標準の日本語入力でもAquaSKKでも発生する。
• Google Chromeでは発生しない。
• MojaveでもHigh Sierraでも発生する。
• Safari 11では発生しないが、Safari 12で発生する。
• Safari Technology PreviewやWebKit Nightlyでも発生する。
• Keyboard Event Viewerで見るかぎりJSレベルではイベントは変化してない。

どうやらWebKitで問題が発生しているらしい。

🛠WebKitのビルド

trunk(問題のあるビルド)

詳しく調べるためにWebKitを手元でビルドした。 Building WebKit | WebKitにいくつかの方法が記載されているが、今回はデバッガを利用したいのでXcodeによるビルドを選択した。

起動して問題が再現することを確認した。

古いWebKit(問題のないビルド)

比較のために問題が再現しないWebKitをビルドする。WebKit Tracからそれっぽいタグを選ぶ。 Safari 11.1.2 (13605.3.8)が問題ないことが分かっているので、Safari-605.3.8 を選択した。 後半の数字と日付がそれっぽいので選んだが、本当に対応してるかはよく分からない。

svn checkout https://svn.webkit.org/repository/webkit/tags/Safari-605.3.8

あとはtrunkと同様にXcodeのワークスペースを開いてビルドする。

👀スタックトレース

☕️ JavaScriptと入力メソッド - みずぴー日記を参考にコードをみてたらTabキーのハンドラみつけた。 これに辿りつくかどうかが肝では、と予想を立てる。

// Soruce/WebCore/page/EventHandler.cpp
void EventHandler::defaultTabEventHandler(KeyboardEvent& event)
{
    Ref<Frame> protectedFrame(m_frame);

    ASSERT(event.type() == eventNames().keydownEvent);

    // We should only advance focus on tabs if no special modifier keys are held down.
    if (event.ctrlKey() || event.metaKey() || event.altGraphKey())
        return;

    Page* page = m_frame.page();
    if (!page)
        return;
    if (!page->tabKeyCyclesThroughElements())
        return;

    FocusDirection focusDirection = event.shiftKey() ? FocusDirectionBackward : FocusDirectionForward;

    // Tabs can be used in design mode editing.
    if (m_frame.document()->inDesignMode())
        return;

    if (page->focusController().advanceFocus(focusDirection, &event))
        event.setDefaultHandled();
}

trunkの defaultTabEventHandler にブレイクポイントを仕掛けて、現在のスタックトレースを入手する。

trunkでのスタックトレース

#0   0x000000059024dbb8 in WebCore::EventHandler::defaultTabEventHandler(WebCore::KeyboardEvent&) at /Volumes/SwiftSSD/webkit/Source/WebCore/page/EventHandler.cpp:3940
#1  0x000000059024da35 in WebCore::EventHandler::defaultKeyboardEventHandler(WebCore::KeyboardEvent&) at /Volumes/SwiftSSD/webkit/Source/WebCore/page/EventHandler.cpp:3470
#2  0x000000058f9b0e24 in WebCore::Node::defaultEventHandler(WebCore::Event&) at /Volumes/SwiftSSD/webkit/Source/WebCore/dom/Node.cpp:2383
#3  0x000000058fc7809d in WebCore::HTMLInputElement::defaultEventHandler(WebCore::Event&) at /Volumes/SwiftSSD/webkit/Source/WebCore/html/HTMLInputElement.cpp:1165
#4  0x000000058f947846 in WebCore::callDefaultEventHandlersInBubblingOrder(WebCore::Event&, WebCore::EventPath const&) at /Volumes/SwiftSSD/webkit/Source/WebCore/dom/EventDispatcher.cpp:61
#5  0x000000058f9472ac in WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&) at /Volumes/SwiftSSD/webkit/Source/WebCore/dom/EventDispatcher.cpp:175
#6  0x000000058f9b05cd in WebCore::Node::dispatchEvent(WebCore::Event&) at /Volumes/SwiftSSD/webkit/Source/WebCore/dom/Node.cpp:2329
#7  0x000000059024cbd4 in WebCore::EventHandler::internalKeyEvent(WebCore::PlatformKeyboardEvent const&) at /Volumes/SwiftSSD/webkit/Source/WebCore/page/EventHandler.cpp:3297
#8  0x000000059024c2d9 in WebCore::EventHandler::keyEvent(WebCore::PlatformKeyboardEvent const&) at /Volumes/SwiftSSD/webkit/Source/WebCore/page/EventHandler.cpp:3163
#9  0x0000000590dd531b in WebCore::UserInputBridge::handleKeyEvent(WebCore::PlatformKeyboardEvent const&, WebCore::InputSource) at /Volumes/SwiftSSD/webkit/Source/WebCore/replay/UserInputBridge.cpp:82
#10 0x0000000588bb3899 in WebKit::handleKeyEvent(WebKit::WebKeyboardEvent const&, WebCore::Page*) at /Volumes/SwiftSSD/webkit/Source/WebKit/WebProcess/WebPage/WebPage.cpp:2465
#11 0x0000000588bb3742 in WebKit::WebPage::keyEvent(WebKit::WebKeyboardEvent const&) at /Volumes/SwiftSSD/webkit/Source/WebKit/WebProcess/WebPage/WebPage.cpp:2476
#12 0x0000000588c5299a in void IPC::callMemberFunctionImpl<WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>, 0ul>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&, std::__1::integer_sequence<unsigned long, 0ul>) at /Volumes/SwiftSSD/webkit/Source/WebKit/Platform/IPC/HandleMessage.h:41
#13 0x0000000588c528f0 in void IPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>, std::__1::integer_sequence<unsigned long, 0ul> >(std::__1::tuple<WebKit::WebKeyboardEvent>&&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&)) at /Volumes/SwiftSSD/webkit/Source/WebKit/Platform/IPC/HandleMessage.h:47
#14 0x0000000588c3da7a in void IPC::handleMessage<Messages::WebPage::KeyEvent, WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&)>(IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&)) at /Volumes/SwiftSSD/webkit/Source/WebKit/Platform/IPC/HandleMessage.h:127
#15 0x0000000588c36142 in WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&) at /Volumes/SwiftSSD/webkit/WebKitBuild/WebKit/Build/Products/Debug/DerivedSources/WebKit2/WebPageMessageReceiver.cpp:213
#16 0x0000000588bba85e in WebKit::WebPage::didReceiveMessage(IPC::Connection&, IPC::Decoder&) at /Volumes/SwiftSSD/webkit/Source/WebKit/WebProcess/WebPage/WebPage.cpp:4038
#17 0x00000005882863ec in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) at /Volumes/SwiftSSD/webkit/Source/WebKit/Platform/IPC/MessageReceiverMap.cpp:123
#18 0x0000000588e37e0d in WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) at /Volumes/SwiftSSD/webkit/Source/WebKit/WebProcess/WebProcess.cpp:645
#19 0x000000058816c93c in IPC::Connection::dispatchMessage(IPC::Decoder&) at /Volumes/SwiftSSD/webkit/Source/WebKit/Platform/IPC/Connection.cpp:940
#20 0x000000058815f998 in IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) at /Volumes/SwiftSSD/webkit/Source/WebKit/Platform/IPC/Connection.cpp:967
#21 0x000000058816d4c4 in IPC::Connection::dispatchOneIncomingMessage() at /Volumes/SwiftSSD/webkit/Source/WebKit/Platform/IPC/Connection.cpp:1036
#22 0x00000005881864a8 in IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()() at /Volumes/SwiftSSD/webkit/Source/WebKit/Platform/IPC/Connection.cpp:933
#23 0x00000005881863b9 in WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call() at /Volumes/SwiftSSD/webkit/WebKitBuild/WebKit/Build/Products/Debug/usr/local/include/wtf/Function.h:101
#24 0x000000059d58b96f in WTF::Function<void ()>::operator()() const at /Volumes/SwiftSSD/webkit/WebKitBuild/WebKit/Build/Products/Debug/usr/local/include/wtf/Function.h:56
#25 0x000000059d5e4103 in WTF::RunLoop::performWork() at /Volumes/SwiftSSD/webkit/Source/WTF/wtf/RunLoop.cpp:106
#26 0x000000059d5e4a94 in WTF::RunLoop::performWork(void*) at /Volumes/SwiftSSD/webkit/Source/WTF/wtf/cf/RunLoopCF.cpp:38

このスタックトレースを参考に、古いWebKitの各所にブレイクポイントを設定して動作を比較する。 その結果、EventDispatcher::dispatchEvent が WebCore::callDefaultEventHandlersInBubblingOrder を呼ぶかどうかの差を発見した。

605.3.8のスタックトレース

#0   0x00000003cee0dc25 in WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&) at /Users/mzp/Safari-605.3.8/Source/WebCore/dom/EventDispatcher.cpp:176
#1  0x00000003cee6d81d in WebCore::Node::dispatchEvent(WebCore::Event&) at /Users/mzp/Safari-605.3.8/Source/WebCore/dom/Node.cpp:2330
#2  0x00000003cf5bfce5 in WebCore::EventHandler::internalKeyEvent(WebCore::PlatformKeyboardEvent const&) at /Users/mzp/Safari-605.3.8/Source/WebCore/page/EventHandler.cpp:3279
#3  0x00000003cf5bf419 in WebCore::EventHandler::keyEvent(WebCore::PlatformKeyboardEvent const&) at /Users/mzp/Safari-605.3.8/Source/WebCore/page/EventHandler.cpp:3145
#4  0x00000003d00bde4b in WebCore::UserInputBridge::handleKeyEvent(WebCore::PlatformKeyboardEvent const&, WebCore::InputSource) at /Users/mzp/Safari-605.3.8/Source/WebCore/replay/UserInputBridge.cpp:83
#5  0x00000003c8a26289 in WebKit::handleKeyEvent(WebKit::WebKeyboardEvent const&, WebCore::Page*) at /Users/mzp/Safari-605.3.8/Source/WebKit/WebProcess/WebPage/WebPage.cpp:2440
#6  0x00000003c8a26132 in WebKit::WebPage::keyEvent(WebKit::WebKeyboardEvent const&) at /Users/mzp/Safari-605.3.8/Source/WebKit/WebProcess/WebPage/WebPage.cpp:2451
#7  0x00000003c8ab982a in void IPC::callMemberFunctionImpl<WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>, 0ul>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&, std::__1::integer_sequence<unsigned long, 0ul>) at /Users/mzp/Safari-605.3.8/Source/WebKit/Platform/IPC/HandleMessage.h:40
#8  0x00000003c8ab9780 in void IPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>, std::__1::integer_sequence<unsigned long, 0ul> >(std::__1::tuple<WebKit::WebKeyboardEvent>&&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&)) at /Users/mzp/Safari-605.3.8/Source/WebKit/Platform/IPC/HandleMessage.h:46
#9  0x00000003c8aa7f66 in void IPC::handleMessage<Messages::WebPage::KeyEvent, WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&)>(IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&)) at /Users/mzp/Safari-605.3.8/Source/WebKit/Platform/IPC/HandleMessage.h:126
#10 0x00000003c8aa1070 in WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&) at /Users/mzp/Safari-605.3.8/WebKitBuild/WebKit/Build/Products/Debug/DerivedSources/WebKit2/WebPageMessageReceiver.cpp:204
#11 0x00000003c8a2cd4e in WebKit::WebPage::didReceiveMessage(IPC::Connection&, IPC::Decoder&) at /Users/mzp/Safari-605.3.8/Source/WebKit/WebProcess/WebPage/WebPage.cpp:3965
#12 0x00000003c824c5e8 in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) at /Users/mzp/Safari-605.3.8/Source/WebKit/Platform/IPC/MessageReceiverMap.cpp:123
#13 0x00000003c8c6e7fd in WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) at /Users/mzp/Safari-605.3.8/Source/WebKit/WebProcess/WebProcess.cpp:648
#14 0x00000003c8141843 in IPC::Connection::dispatchMessage(IPC::Decoder&) at /Users/mzp/Safari-605.3.8/Source/WebKit/Platform/IPC/Connection.cpp:907
#15 0x00000003c81369d8 in IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) at /Users/mzp/Safari-605.3.8/Source/WebKit/Platform/IPC/Connection.cpp:934
#16 0x00000003c8141e94 in IPC::Connection::dispatchOneMessage() at /Users/mzp/Safari-605.3.8/Source/WebKit/Platform/IPC/Connection.cpp:965
#17 0x00000003c8159add in IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()() at /Users/mzp/Safari-605.3.8/Source/WebKit/Platform/IPC/Connection.cpp:901
#18 0x00000003c8159a39 in WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call() at /Users/mzp/Safari-605.3.8/WebKitBuild/WebKit/Build/Products/Debug/usr/local/include/wtf/Function.h:101
#19 0x00000003dd2f9a9b in WTF::Function<void ()>::operator()() const at /Users/mzp/Safari-605.3.8/WebKitBuild/WebKit/Build/Products/Debug/usr/local/include/wtf/Function.h:56
#20 0x00000003dd33e5f3 in WTF::RunLoop::performWork() at /Users/mzp/Safari-605.3.8/Source/WTF/wtf/RunLoop.cpp:106

🔍コード確認

EventDispatcher::dispatchEvent を確認すると、callDefaultEventHandlersInBubblingOrder にはifによるガードがついている。

// WebCore/dom/EventDispatcher.cpp
    if (!event.defaultPrevented() && !event.defaultHandled()) {
        // FIXME: Not clear why we need to reset the target for the default event handlers.
        // We should research this, and remove this code if possible.
        auto* finalTarget = event.target();
        event.setTarget(EventPath::eventTargetRespectingTargetRules(node));
        callDefaultEventHandlersInBubblingOrder(event, eventPath);
        event.setTarget(finalTarget);
    }

デバッガを使って条件節の値を確認する。 すると、古いSafariではevent.defaultHandled() が trueだが、trunkではevent.defaultHandled() が falseになっている。

コードを目で比較すると、trunkでは resetBeforeDispatch() の呼び出しがある。

// WebCore/dom/EventDispatcher.cppASSERT_WITH_SECURITY_IMPLICATION(ScriptDisallowedScope::InMainThread::isEventDispatchAllowedInSubtree(node));
    
    LOG(Events, "EventDispatcher::dispatchEvent %s on node %s", event.type().string().utf8().data(), node.nodeName().utf8().data());

    auto protectedNode = makeRef(node);
    auto protectedView = makeRefPtr(node.document().view());

    EventPath eventPath { node, event };

    ChildNodesLazySnapshot::takeChildNodesLazySnapshot();

    event.resetBeforeDispatch();  // <- ??????

コードを確認すると、defaultHandled を初期化している。

void Event::resetBeforeDispatch()
{
    m_defaultHandled = false;
}

なので日本語入力とTabキーの問題を追っていたが、真の問題は入力メソッドが処理したキー入力に対してもデフォルトのハンドラが起動することらしい。

📖コミット調査

trackのblame機能を使って、どのコミットでこれが増えたか特定する。

r228260が原因らしい。 コミットログによると、イベント配信間の影響を切るために導入されているらしい。

(WebCore::Event::resetBeforeDispatch): Added. Clears m_defaultHandled so
a value left over from a previous dispatch doesn't affect the next dispatch.
----
(WebCore::Event::resetBeforeDispatch): 追加。以前のイベント配信が次の配信に影響しないようにm_defaultHandledをリセットする。

🗣バグレポート

adhocに直す方法はいくつかあるが、どれを選択するのがいいか分からない。 WebKitとRadarにレポートした。どう書くか迷ったがサマリーでは具体的に困ってることを書いて、真の問題はdetailに書くことにした。 またタイトルにregressionというワードを含めた。

• 188370 – Events handled by input method invoke default event handler[regression]
• rdar://42991694: Safari moves focus by tab key during composing text by input method.

その結果、以下のような修正が適用された。入力メソッドが処理したことを示す isDefaultEventHandlerIgnored() 関数が追加された。

Index: trunk/Source/WebCore/dom/Event.h
===================================================================
--- a/trunk/Source/WebCore/dom/Event.h
+++ b/trunk/Source/WebCore/dom/Event.h
@@ -120,4 +120,7 @@
     void setDefaultHandled() { m_defaultHandled = true; }
 
+    bool isDefaultEventHandlerIgnored() const { return m_isDefaultEventHandlerIgnored; }
+    void setIsDefaultEventHandlerIgnored() { m_isDefaultEventHandlerIgnored = true; }
+
     void setInPassiveListener(bool value) { m_isExecutingPassiveEventListener = value; }
 
@@ -157,4 +160,5 @@
     bool m_wasCanceled { false };
     bool m_defaultHandled { false };
+    bool m_isDefaultEventHandlerIgnored { false };
     bool m_isTrusted { false };
     bool m_isExecutingPassiveEventListener { false };

Index: trunk/Source/WebCore/dom/EventDispatcher.cpp
===================================================================
--- a/trunk/Source/WebCore/dom/EventDispatcher.cpp
+++ b/trunk/Source/WebCore/dom/EventDispatcher.cpp
@@ -168,5 +168,5 @@
     // default handling, the detail of which handlers are called is an internal
     // implementation detail and not part of the DOM.
-    if (!event.defaultPrevented() && !event.defaultHandled()) {
+    if (!event.defaultPrevented() && !event.defaultHandled() && !event.isDefaultEventHandlerIgnored()) {
         // FIXME: Not clear why we need to reset the target for the default event handlers.
         // We should research this, and remove this code if possible.

Index: trunk/Source/WebCore/page/EventHandler.cpp
===================================================================
--- a/trunk/Source/WebCore/page/EventHandler.cpp
+++ b/trunk/Source/WebCore/page/EventHandler.cpp
@@ -3289,5 +3289,5 @@
         keydown = KeyboardEvent::create(keyDownEvent, &m_frame.windowProxy());
         keydown->setTarget(element);
-        keydown->setDefaultHandled();
+        keydown->setIsDefaultEventHandlerIgnored();
     }