Bug 215622

Summary: REGRESSION(r265092): delegatesFocus causes WebKit to crash
Product: WebKit Reporter: Liam DeBeasi <ldebeasi>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: caleb.d.williams, cdumez, gsnedders, koivisto, lizmitchell, niklasmerz, overbek, rniwa, webkit-bug-importer, webkitbugzilla, wenson_hsieh, youennf
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: All   
OS: Other   
Bug Depends on:    
Bug Blocks: 148695    
Attachments:
Description Flags
Code Reproduction
none
Fixes the crash
youennf: review+
Crash with focusable button none

Liam DeBeasi
Reported 2020-08-18 13:39:14 PDT
Created attachment 406804 [details] Code Reproduction As of iOS 14 beta 5 and STP 112, clicking an element that has delegatesFocus set on its shadow root causes WebKit to crash. This only happens when there is no element inside of the component to forward focus to. Adding a focusable element, such as an input, fixes the issue. Steps to reproduce: 1. Open attached reproduction file on a device running iOS 14 beta 5 or STP 112. 2. Click/Tap the "Clicking me will cause a crash" text. 3. Notice that the browser reloads. Clicking again gives the "A problem repeatedly occurred" error. 4. Click/Tap the "Clicking me will NOT cause a crash" text. 5. Notice that the browser does NOT reload. This is because there is an input that delegatesFocus can forward focus to. Expected Behavior: I would expect that WebKit does not crash when clicking an element with delegatesFocus. Actual Behavior: WebKit crashes when clicking an element with delegatesFocus.
Attachments
Code Reproduction (715 bytes, text/html)
2020-08-18 13:39 PDT, Liam DeBeasi 		no flags
Details
Fixes the crash (3.93 KB, patch)
2020-08-18 23:56 PDT, Ryosuke Niwa 		youennf: review+
DetailsFormatted DiffDiff
Crash with focusable button (804 bytes, text/html)
2020-08-24 10:07 PDT, Elizabeth Mitchell 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Sam Sneddon [:gsnedders]
Comment 1 2020-08-18 14:08:33 PDT
r265092 looks suspicious here. Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebCore 0x00007fff3a81ff5b WebCore::EventHandler::dispatchMouseEvent(WTF::AtomString const&, WebCore::Node*, int, WebCore::PlatformMouseEvent const&, WebCore::EventHandler::FireMouseOverOut) + 2683 1 com.apple.WebCore 0x00007fff38f45e6f WebCore::EventHandler::handleMousePressEvent(WebCore::PlatformMouseEvent const&) + 3327 2 com.apple.WebKit 0x00007fff3b5cac3b WebKit::WebPage::mouseEvent(WebKit::WebMouseEvent const&) + 451 3 com.apple.WebKit 0x00007fff3bbbe127 WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&) + 11385 4 com.apple.WebKit 0x00007fff3b64a075 IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) + 125 5 com.apple.WebKit 0x00007fff3baa123a WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 28 is the top of the crash
Radar WebKit Bug Importer
Comment 2 2020-08-18 14:08:47 PDT
<rdar://problem/67351846>
Ryosuke Niwa
Comment 3 2020-08-18 23:56:28 PDT
Created attachment 406830 [details] Fixes the crash
Ryosuke Niwa
Comment 4 2020-08-19 11:14:52 PDT
Committed r265881: <https://trac.webkit.org/changeset/265881>
Alexey Proskuryakov
Comment 5 2020-08-22 16:10:04 PDT
*** Bug 215732 has been marked as a duplicate of this bug. ***
Elizabeth Mitchell
Comment 6 2020-08-24 10:07:40 PDT
Created attachment 407108 [details] Crash with focusable button Following up on this bug, STP 112 also crashed when clicking an element in a component that had a focusable element (a button). I'm curious if the implementation does not consider the button element focusable. After the patch, does clicking the text in the new attachment focus the button element?
Ryosuke Niwa
Comment 7 2020-08-24 11:37:26 PDT
(In reply to Liz Mitchell from comment #6) > Created attachment 407108 [details] > Crash with focusable button > > Following up on this bug, STP 112 also crashed when clicking an element in a > component that had a focusable element (a button). > > I'm curious if the implementation does not consider the button element > focusable. After the patch, does clicking the text in the new attachment > focus the button element? Buttons are not focusable in macOS / iOS WebKit per OS convention. That's the bug 118043.
Elizabeth Mitchell
Comment 8 2020-09-18 12:47:47 PDT
Safari 14.0 (15610.1.28.1.9) on Desktop released September 16, 2020 does not have this fix applied. This is causing production breakages with our components on Safari for users that upgrade from Safari 13 to 14 with the latest OS X updates.
Ryosuke Niwa
Comment 9 2020-09-28 17:11:34 PDT
(In reply to Liz Mitchell from comment #8) > Safari 14.0 (15610.1.28.1.9) on Desktop released September 16, 2020 does not > have this fix applied. > > This is causing production breakages with our components on Safari for users > that upgrade from Safari 13 to 14 with the latest OS X updates. Please try the latest beta of macOS Big Sir or Safari 14.
Elizabeth Mitchell
Comment 10 2020-09-29 12:13:26 PDT
(In reply to Ryosuke Niwa from comment #9) > Please try the latest beta of macOS Big Sir or Safari 14. I'm unable to update past macOS Big Sur due to corp policy right now, but Safari TP Release 113 on macOS Catalina was confirmed to resolve this issue. The problem is that a recent app store update to Safari on Catalina updated Safari from 13 to 14 without the latest WebKit changes (pre-Release 113). This has caused multiple users to report production breakages as macOS Catalina updates their Safari app to v14 with this regression.
Ryosuke Niwa
Comment 11 2020-09-29 12:18:25 PDT
(In reply to Liz Mitchell from comment #10) > (In reply to Ryosuke Niwa from comment #9) > > Please try the latest beta of macOS Big Sir or Safari 14. > > I'm unable to update past macOS Big Sur due to corp policy right now, but > Safari TP Release 113 on macOS Catalina was confirmed to resolve this issue. > > The problem is that a recent app store update to Safari on Catalina updated > Safari from 13 to 14 without the latest WebKit changes (pre-Release 113). > > This has caused multiple users to report production breakages as macOS > Catalina updates their Safari app to v14 with this regression. I understand and we're aware of the issue. We're not going to recede Safari 14 for this issue alone though. Having said that, what ships to macOS Big Sur will ship to macOS Catalina and macOS Mojave.
Smoley
Comment 12 2020-10-28 11:29:01 PDT
*** Bug 218290 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.