<rdar://problem/48020610>
My current thinking is that I can apply the same strategy in WebPage::completeSyntheticClick: if (newFocusedElement && newFocusedElement == oldFocusedElement) elementDidRefocus(*newFocusedElement); ...to either WebPage::dispatchTouchEvent or WebPage::touchEventSync as well, but only if default behavior was prevented. While this approach does fix this bug, it's eerily similar to code that was added in <http://trac.webkit.org/r167774> (in dispatchTouchEvent), which subsequently caused <rdar://problem/22126518> and was later reverted in <https://trac.webkit.org/r188405>. Strictly speaking, my current approach wouldn't reintroduce <rdar://problem/22126518> because it would additionally restrict the focused element update to the case where preventDefault() was called...but I still fear that it will introduce similar bugs where touching the page might summon input views against users' expectation. An additional thought I had was that maybe we could only apply this refocusing logic if the touch happens to be over the already focused element. But then I realized that this wouldn't be sufficient to fix my bug, since the web pages in question redirect focus to an offscreen hidden editable element that we wouldn't hit-test to anyways. ...in other words, the case where the input view might show up against user expectation seems to be indistinguishable from the case where we actually want to show an input view :( Maybe... - The only safe way to fix this is to put the behavior behind a quirk or: - I'm being overly concerned about keyboards unexpectedly popping up, and we should just try this and see what happens.........and then put it behind a quirk if we discover any breakage on real sites :/
Created attachment 364901 [details] Patch
> or: > > - I'm being overly concerned about keyboards unexpectedly popping up, and we > should just try this and see what happens.........and then put it behind a > quirk if we discover any breakage on real sites :/ going to live on this for a bit and see if I encounter any surprise keyboards.
Comment on attachment 364901 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364901&action=review > Source/WebKit/WebProcess/WebPage/WebPage.cpp:2821 > + auto* oldFocusedFrame = m_page->focusController().focusedFrame(); > + auto* oldFocusedElement = oldFocusedFrame ? oldFocusedFrame->document()->focusedElement() : nullptr; Please use Ref/RefPtr. > Source/WebKit/WebProcess/WebPage/WebPage.cpp:2830 > + auto* newFocusedFrame = m_page->focusController().focusedFrame(); > + auto* newFocusedElement = newFocusedFrame ? newFocusedFrame->document()->focusedElement() : nullptr; Ditto
Created attachment 364941 [details] Patch for landing
Comment on attachment 364901 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364901&action=review >> Source/WebKit/WebProcess/WebPage/WebPage.cpp:2821 >> + auto* oldFocusedElement = oldFocusedFrame ? oldFocusedFrame->document()->focusedElement() : nullptr; > > Please use Ref/RefPtr. Fixed! (here and below)
Comment on attachment 364941 [details] Patch for landing Clearing flags on attachment: 364941 Committed r243044: <https://trac.webkit.org/changeset/243044>