WebKit Bugzilla
Attachment 372787 Details for
Bug 198928
: m_focusedElement != &element in WebPage::elementDidBlur() sometimes; nonsense blur event fired at frame
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
To land
bug-198928-20190624124647.patch (text/plain), 11.10 KB, created by
Daniel Bates
on 2019-06-24 12:46:48 PDT
(
hide
)
Description:
To land
Filename:
MIME Type:
Creator:
Daniel Bates
Created:
2019-06-24 12:46:48 PDT
Size:
11.10 KB
patch
obsolete
>Subversion Revision: 246754 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 98ed8be8cf36b8bb33d622eae1c26acd39463733..a891cdceba038b3d3a9708edb8821ffc12d8f942 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,51 @@ >+2019-06-24 Daniel Bates <dabates@apple.com> >+ >+ m_focusedElement != &element in WebPage::elementDidBlur() sometimes >+ https://bugs.webkit.org/show_bug.cgi?id=198928 >+ <rdar://problem/51814327> >+ >+ Reviewed by Brent Fulgham. >+ >+ This can happen when the focused editable element is inside a nested frame and a person >+ taps outside that frame. For reasons that seem lost to time, WebKit2 on iOS would mutate >+ the focused frame whenever computing selection positioning information (say, for a tap). >+ This "quirk" was added in r163476, but that code has go through multiple iterations and >+ is no longer comparable to the current code. Yet, the original mutation of the focused >+ frame remained. As a result the UI process and Web process go out of sync with respect >+ to what each thinks is the focused element and this visually manifest itself in at least >+ two ways: >+ >+ 1. A non-sensical DOM focus event would be dispatched at the frame tapped, but >+ we would keep the focused element focused. >+ >+ 2. Because we would keep the focused element focused in (1), even though its frame >+ is not focused, the keyboard would be active (software keyboard on screen or candidate bar >+ on screen if a hardware keyboard is attached), but appear unresponsive: any keys pressed >+ would not cause text to be typed into the editable field. >+ >+ Because of (1) it was possible for m_focusedElement != &element in WebPage::elementDidBlur(). >+ When this happens the UI process would never receive an ElementDidBlur message and hence would >+ not clear out the focused element state and hide the keyboard. >+ >+ We neither do this frame focus mutation in Legacy WebKit on iOS nor Mac. Let's remove this quirk. >+ If it turns out that it causes a compatibility issue then we will be in a better position to >+ understand its purpose and consider bringing this quirk back, if needed. >+ >+ * Shared/ios/InteractionInformationRequest.cpp: >+ (WebKit::InteractionInformationRequest::encode const): >+ (WebKit::InteractionInformationRequest::decode): >+ (WebKit::InteractionInformationRequest::isValidForRequest): >+ (WebKit::InteractionInformationRequest::isApproximatelyValidForRequest): >+ * Shared/ios/InteractionInformationRequest.h: >+ * UIProcess/API/Cocoa/WKWebView.mm: >+ (-[WKWebView _requestActivatedElementAtPosition:completionBlock:]): >+ * UIProcess/ios/WKContentViewInteraction.mm: >+ (-[WKContentView _webTouchEventsRecognized:]): >+ Remove the readOnly field. >+ >+ * WebProcess/WebPage/ios/WebPageIOS.mm: >+ (WebKit::selectionPositionInformation): Remove code to mutate the focused frame. >+ > 2019-06-24 Per Arne Vollan <pvollan@apple.com> > > [Cocoa] Avoid creating a PlatformMediaSessionManager when the WebProcess is suspended or resumed >diff --git a/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp b/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp >index 3d704e6e6e6da38a8bc7dd4021af1e1b5343625b..0bd841619f748c34fab94ebc2da626cbeb560c9d 100644 >--- a/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp >+++ b/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp >@@ -39,7 +39,6 @@ void InteractionInformationRequest::encode(IPC::Encoder& encoder) const > encoder << includeSnapshot; > encoder << includeLinkIndicator; > encoder << linkIndicatorShouldHaveLegacyMargins; >- encoder << readonly; > } > > bool InteractionInformationRequest::decode(IPC::Decoder& decoder, InteractionInformationRequest& result) >@@ -56,9 +55,6 @@ bool InteractionInformationRequest::decode(IPC::Decoder& decoder, InteractionInf > if (!decoder.decode(result.linkIndicatorShouldHaveLegacyMargins)) > return false; > >- if (!decoder.decode(result.readonly)) >- return false; >- > return true; > } > >@@ -76,9 +72,6 @@ bool InteractionInformationRequest::isValidForRequest(const InteractionInformati > if (other.linkIndicatorShouldHaveLegacyMargins != linkIndicatorShouldHaveLegacyMargins) > return false; > >- if (!other.readonly && readonly) >- return false; >- > return true; > } > >@@ -90,9 +83,6 @@ bool InteractionInformationRequest::isApproximatelyValidForRequest(const Interac > if (other.includeLinkIndicator && !includeLinkIndicator) > return false; > >- if (!other.readonly && readonly) >- return false; >- > if (other.linkIndicatorShouldHaveLegacyMargins != linkIndicatorShouldHaveLegacyMargins) > return false; > >diff --git a/Source/WebKit/Shared/ios/InteractionInformationRequest.h b/Source/WebKit/Shared/ios/InteractionInformationRequest.h >index b318b40e31ae712aedb7140f91a20df3834df4bb..9e2cfcf5d8b4b3819e841ca99198992127292ecb 100644 >--- a/Source/WebKit/Shared/ios/InteractionInformationRequest.h >+++ b/Source/WebKit/Shared/ios/InteractionInformationRequest.h >@@ -44,11 +44,6 @@ struct InteractionInformationRequest { > > bool linkIndicatorShouldHaveLegacyMargins { false }; > >- // FIXME: This readonly flag ought to be true by default, but there are a number of interactions (e.g. selection) that currently >- // rely on the fact that interaction information requests additionally change the focused frame. We should explicitly turn the >- // readonly bit off in these scenarios, and make sure that all other position information requests don't move focus around. >- bool readonly { false }; >- > InteractionInformationRequest() { } > explicit InteractionInformationRequest(WebCore::IntPoint point) > { >diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >index bbeaef4865e1da30bdf1ac443adee07108f86397..c77c25ced57de5a7157695df38e85ce13b16cb73 100644 >--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >@@ -6675,7 +6675,6 @@ - (void)_requestActivatedElementAtPosition:(CGPoint)position completionBlock:(vo > { > auto infoRequest = WebKit::InteractionInformationRequest(WebCore::roundedIntPoint(position)); > infoRequest.includeSnapshot = true; >- infoRequest.readonly = true; > > [_contentView doAfterPositionInformationUpdate:[capturedBlock = makeBlockPtr(block)] (WebKit::InteractionInformationAtPosition information) { > capturedBlock([_WKActivatedElementInfo activatedElementInfoWithInteractionInformationAtPosition:information]); >diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm >index 702ea88737ccd606726277d52e64866838adc453..53b0be997b51aa919962cfa44c5b52c903863bfa 100644 >--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm >+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm >@@ -1316,7 +1316,6 @@ - (void)_webTouchEventsRecognized:(UIWebTouchEventsGestureRecognizer *)gestureRe > _layerTreeTransactionIdAtLastTouchStart = downcast<WebKit::RemoteLayerTreeDrawingAreaProxy>(*_page->drawingArea()).lastCommittedLayerTreeTransactionID(); > > WebKit::InteractionInformationRequest positionInformationRequest { WebCore::IntPoint(_lastInteractionLocation) }; >- positionInformationRequest.readonly = true; > [self doAfterPositionInformationUpdate:[assistant = WeakObjCPtr<WKActionSheetAssistant>(_actionSheetAssistant.get())] (WebKit::InteractionInformationAtPosition information) { > [assistant interactionDidStartWithPositionInformation:information]; > } forRequest:positionInformationRequest]; >diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm >index ad17b6e73ac7f2ab603193fbefd6f9aa1275ccaf..044e11f017876901c81d39124bf68fde38e1cfa0 100644 >--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm >+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm >@@ -2620,9 +2620,6 @@ static void selectionPositionInformation(WebPage& page, const InteractionInforma > return; > > RenderObject* renderer = hitNode->renderer(); >- if (!request.readonly) >- page.corePage()->focusController().setFocusedFrame(result.innerNodeFrame()); >- > info.bounds = renderer->absoluteBoundingBoxRect(true); > // We don't want to select blocks that are larger than 97% of the visible area of the document. > if (is<HTMLAttachmentElement>(*hitNode)) { >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 4f111d89d98287ddb5838662ce4b2d925e3d67f7..668f5d1d7137ac25574448b472a082266c014887 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,18 @@ >+2019-06-24 Daniel Bates <dabates@apple.com> >+ >+ m_focusedElement != &element in WebPage::elementDidBlur() sometimes >+ https://bugs.webkit.org/show_bug.cgi?id=198928 >+ <rdar://problem/51814327> >+ >+ Reviewed by Brent Fulgham. >+ >+ Update test result now that we do not mutate the focused frame on tap. >+ >+ * TestExpectations: Skip problematic test editing/deleting/smart-delete-paragraph-003.html; >+ See <https://bugs.webkit.org/show_bug.cgi?id=198928#c16>, <https://bugs.webkit.org/show_bug.cgi?id=198928#c17>, >+ and <https://bugs.webkit.org/show_bug.cgi?id=199039> for more details. >+ * fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt: >+ > 2019-06-24 Antoine Quint <graouts@apple.com> > > [Pointer Events WPT] Unskip imported/w3c/web-platform-tests/pointerevents/pointerlock/pointerevent_coordinates_when_locked.html >diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations >index e73d995fae93fef508a2138ad83e1d8dace3c7dd..126fd018691b2dabfe6c249d4bc3544ec0250080 100644 >--- a/LayoutTests/TestExpectations >+++ b/LayoutTests/TestExpectations >@@ -3438,3 +3438,5 @@ imported/w3c/web-platform-tests/websockets/Secure-Send-unpaired-surrogates.any.w > > # iOS only > fast/dom/linkify-phone-numbers.html [ ImageOnlyFailure ] >+ >+webkit.org/b/199039 editing/deleting/smart-delete-paragraph-003.html [ Skip ] >diff --git a/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt b/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt >index 906fc2538b441a9b260bae61628f8f96c0e293f9..806179fe54572413f047f886c483e59341b601b9 100644 >--- a/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt >+++ b/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt >@@ -5,7 +5,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE > > dispatched blur to main frame > dispatched focus to <input> >-dispatched focus to main frame >+dispatched blur to <input> > PASS dismissed form accessory > PASS successfullyParsed is true >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 198928
:
372379
|
372387
|
372406
| 372787