- Revision
- 275054
- Author
- [email protected]
- Date
- 2021-03-25 13:22:10 -0700 (Thu, 25 Mar 2021)
Log Message
[iOS] Date picker view jumps to center when switching between MobileSafari and another app
https://bugs.webkit.org/show_bug.cgi?id=223662
<rdar://problem/74748727>
Reviewed by Tim Horton.
Tapping on a date input presents a UIDatePicker inside of a
UIContextMenuInteraction. The presentation of context menus is
dependent on the _positionInformation in WKContentViewInteraction being
up-to-date.
To ensure the value is up-to-date, a position information request is
made prior to presenting the date picker. However, the location used
for the request is the current FocusedElementInformation's
lastInteractionLocation. This is fine in most cases, since the date
picker is usually present immediately after tapping on the date input.
However, the date picker can be closed and presented again when
switching between MobileSafari and another app, as the input is focused
again when MobileSafari is reopened. If the switch is performed using a
gesture, the last interaction location is modified. Consequently, the
last interaction location does not always correspond to the position
of the date input, and the position information request gives us
incorrect information, leading to incorrect presentation of the picker.
To fix, add a new way of presenting context menus that is not dependent
on position information. Instead, form controls can use the information
they already have in FocusedElementInformation to present their
peripherals.
No new tests, since this bug only reproduces when swiping to switch apps.
Other touches in the web view immediately dismiss the date picker, making
the bug difficult to reproduce in a test.
* Shared/FocusedElementInformation.cpp:
(WebKit::FocusedElementInformation::encode const):
(WebKit::FocusedElementInformation::decode):
* Shared/FocusedElementInformation.h:
Added a member to inform FocusedElementInformation whether the focused
element is in a subscrollable region.
See r248447 for more details on why this information is needed when
creating a UITargetedPreview.
* UIProcess/ios/WKContentViewInteraction.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView overridePositionTrackingViewForTargetedPreviewIfNecessary:containerScrollingNodeID:]):
Factored out the logic that overrides the targeted preview's position
tracking view when the element is in a subscrollable region.
(-[WKContentView _createTargetedContextMenuHintPreviewForFocusedElement]):
Create a targeted preview using the current FocusedElementInformation.
(-[WKContentView _createTargetedContextMenuHintPreviewIfPossible]):
* UIProcess/ios/forms/WKDateTimeInputControl.mm:
(-[WKDateTimePicker controlBeginEditing]):
Displaying the context menu no longer needs to be done asynchronously
since the FocusedElementInformation is already available during
controlBeginEditing.
* UIProcess/ios/forms/WKFormSelectPicker.mm:
Adopt the new logic for <select> elements, since the presented context
menu is an input peripheral, and can run into the same issues as the
date picker.
(-[WKSelectPicker controlBeginEditing]):
(-[WKSelectPicker contextMenuInteraction:previewForHighlightingMenuWithConfiguration:]):
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::completeSyntheticClick):
Remove a redundant call to elementDidRefocus following r258333. The
FocusController already calls elementDidRefocus before this point, when
the mousepress event is dispatched.
The redundant call was uncovered when making the context menu
presentation synchronous, and causes problems with the presentation.
Specifically, the first call to elementDidRefocus causes the context
menu to be presented. The second call then results in an attempt
to scroll the web view, since the context menu can obscure the
assisted node. The additional scrolling is unwanted behavior, since it
does not occur when focusing the node for the first time.
This issue was previously unobserved, since the asynchronous presentation
of the context menu (after obtaining new position information) resulted
in both calls to elementDidRefocus occuring before presentation.
(WebKit::WebPage::getFocusedElementInformation):
Forward the scrolling node information to the UIProcess, so that the
position tracking view of the UITargetedPreview can be overridden
if necessary.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (275053 => 275054)
--- trunk/Source/WebKit/ChangeLog 2021-03-25 20:20:47 UTC (rev 275053)
+++ trunk/Source/WebKit/ChangeLog 2021-03-25 20:22:10 UTC (rev 275054)
@@ -1,3 +1,101 @@
+2021-03-25 Aditya Keerthi <[email protected]>
+
+ [iOS] Date picker view jumps to center when switching between MobileSafari and another app
+ https://bugs.webkit.org/show_bug.cgi?id=223662
+ <rdar://problem/74748727>
+
+ Reviewed by Tim Horton.
+
+ Tapping on a date input presents a UIDatePicker inside of a
+ UIContextMenuInteraction. The presentation of context menus is
+ dependent on the _positionInformation in WKContentViewInteraction being
+ up-to-date.
+
+ To ensure the value is up-to-date, a position information request is
+ made prior to presenting the date picker. However, the location used
+ for the request is the current FocusedElementInformation's
+ lastInteractionLocation. This is fine in most cases, since the date
+ picker is usually present immediately after tapping on the date input.
+ However, the date picker can be closed and presented again when
+ switching between MobileSafari and another app, as the input is focused
+ again when MobileSafari is reopened. If the switch is performed using a
+ gesture, the last interaction location is modified. Consequently, the
+ last interaction location does not always correspond to the position
+ of the date input, and the position information request gives us
+ incorrect information, leading to incorrect presentation of the picker.
+
+ To fix, add a new way of presenting context menus that is not dependent
+ on position information. Instead, form controls can use the information
+ they already have in FocusedElementInformation to present their
+ peripherals.
+
+ No new tests, since this bug only reproduces when swiping to switch apps.
+ Other touches in the web view immediately dismiss the date picker, making
+ the bug difficult to reproduce in a test.
+
+ * Shared/FocusedElementInformation.cpp:
+ (WebKit::FocusedElementInformation::encode const):
+ (WebKit::FocusedElementInformation::decode):
+ * Shared/FocusedElementInformation.h:
+
+ Added a member to inform FocusedElementInformation whether the focused
+ element is in a subscrollable region.
+
+ See r248447 for more details on why this information is needed when
+ creating a UITargetedPreview.
+
+ * UIProcess/ios/WKContentViewInteraction.h:
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView overridePositionTrackingViewForTargetedPreviewIfNecessary:containerScrollingNodeID:]):
+
+ Factored out the logic that overrides the targeted preview's position
+ tracking view when the element is in a subscrollable region.
+
+ (-[WKContentView _createTargetedContextMenuHintPreviewForFocusedElement]):
+
+ Create a targeted preview using the current FocusedElementInformation.
+
+ (-[WKContentView _createTargetedContextMenuHintPreviewIfPossible]):
+ * UIProcess/ios/forms/WKDateTimeInputControl.mm:
+ (-[WKDateTimePicker controlBeginEditing]):
+
+ Displaying the context menu no longer needs to be done asynchronously
+ since the FocusedElementInformation is already available during
+ controlBeginEditing.
+
+ * UIProcess/ios/forms/WKFormSelectPicker.mm:
+
+ Adopt the new logic for <select> elements, since the presented context
+ menu is an input peripheral, and can run into the same issues as the
+ date picker.
+
+ (-[WKSelectPicker controlBeginEditing]):
+ (-[WKSelectPicker contextMenuInteraction:previewForHighlightingMenuWithConfiguration:]):
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::completeSyntheticClick):
+
+ Remove a redundant call to elementDidRefocus following r258333. The
+ FocusController already calls elementDidRefocus before this point, when
+ the mousepress event is dispatched.
+
+ The redundant call was uncovered when making the context menu
+ presentation synchronous, and causes problems with the presentation.
+ Specifically, the first call to elementDidRefocus causes the context
+ menu to be presented. The second call then results in an attempt
+ to scroll the web view, since the context menu can obscure the
+ assisted node. The additional scrolling is unwanted behavior, since it
+ does not occur when focusing the node for the first time.
+
+ This issue was previously unobserved, since the asynchronous presentation
+ of the context menu (after obtaining new position information) resulted
+ in both calls to elementDidRefocus occuring before presentation.
+
+ (WebKit::WebPage::getFocusedElementInformation):
+
+ Forward the scrolling node information to the UIProcess, so that the
+ position tracking view of the UITargetedPreview can be overridden
+ if necessary.
+
2021-03-25 BJ Burg <[email protected]>
SendKeys on Input of type=file returns element not found in some cases
Modified: trunk/Source/WebKit/Shared/FocusedElementInformation.cpp (275053 => 275054)
--- trunk/Source/WebKit/Shared/FocusedElementInformation.cpp 2021-03-25 20:20:47 UTC (rev 275053)
+++ trunk/Source/WebKit/Shared/FocusedElementInformation.cpp 2021-03-25 20:22:10 UTC (rev 275054)
@@ -99,6 +99,7 @@
encoder << label;
encoder << ariaLabel;
encoder << focusedElementIdentifier;
+ encoder << containerScrollingNodeID;
#if ENABLE(DATALIST_ELEMENT)
encoder << hasSuggestions;
encoder << isFocusingWithDataListDropdown;
@@ -225,6 +226,9 @@
if (!decoder.decode(result.focusedElementIdentifier))
return false;
+ if (!decoder.decode(result.containerScrollingNodeID))
+ return false;
+
#if ENABLE(DATALIST_ELEMENT)
if (!decoder.decode(result.hasSuggestions))
return false;
Modified: trunk/Source/WebKit/Shared/FocusedElementInformation.h (275053 => 275054)
--- trunk/Source/WebKit/Shared/FocusedElementInformation.h 2021-03-25 20:20:47 UTC (rev 275053)
+++ trunk/Source/WebKit/Shared/FocusedElementInformation.h 2021-03-25 20:22:10 UTC (rev 275054)
@@ -141,6 +141,7 @@
bool isFocusingWithValidationMessage { false };
FocusedElementIdentifier focusedElementIdentifier { 0 };
+ WebCore::ScrollingNodeID containerScrollingNodeID { 0 };
void encode(IPC::Encoder&) const;
static WARN_UNUSED_RETURN bool decode(IPC::Decoder&, FocusedElementInformation&);
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (275053 => 275054)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h 2021-03-25 20:20:47 UTC (rev 275053)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h 2021-03-25 20:22:10 UTC (rev 275054)
@@ -672,6 +672,7 @@
- (void)setContinuousSpellCheckingEnabled:(BOOL)enabled;
#if USE(UICONTEXTMENU)
+- (UITargetedPreview *)_createTargetedContextMenuHintPreviewForFocusedElement;
- (UITargetedPreview *)_createTargetedContextMenuHintPreviewIfPossible;
- (void)_removeContextMenuViewIfPossible;
#endif
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (275053 => 275054)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2021-03-25 20:20:47 UTC (rev 275053)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2021-03-25 20:22:10 UTC (rev 275054)
@@ -8238,6 +8238,29 @@
return adoptNS([[UITargetedPreview alloc] initWithView:snapshotView parameters:parameters.get() target:target.get()]);
}
+- (void)overridePositionTrackingViewForTargetedPreviewIfNecessary:(UITargetedPreview *)targetedPreview containerScrollingNodeID:(WebCore::ScrollingNodeID)scrollingNodeID
+{
+ if (!scrollingNodeID)
+ return;
+
+ UIScrollView *positionTrackingView = self.webView.scrollView;
+ if (auto* scrollingCoordinator = _page->scrollingCoordinatorProxy())
+ positionTrackingView = scrollingCoordinator->scrollViewForScrollingNodeID(scrollingNodeID);
+
+ if ([targetedPreview respondsToSelector:@selector(_setOverridePositionTrackingView:)])
+ [targetedPreview _setOverridePositionTrackingView:positionTrackingView];
+}
+
+- (UITargetedPreview *)_createTargetedContextMenuHintPreviewForFocusedElement
+{
+ RetainPtr<UITargetedPreview> targetedPreview = createFallbackTargetedPreview(self, self.containerForContextMenuHintPreviews, _focusedElementInformation.interactionRect);
+
+ [self overridePositionTrackingViewForTargetedPreviewIfNecessary:targetedPreview.get() containerScrollingNodeID:_focusedElementInformation.containerScrollingNodeID];
+
+ _contextMenuInteractionTargetedPreview = WTFMove(targetedPreview);
+ return _contextMenuInteractionTargetedPreview.get();
+}
+
- (UITargetedPreview *)_createTargetedContextMenuHintPreviewIfPossible
{
RetainPtr<UITargetedPreview> targetedPreview;
@@ -8255,15 +8278,8 @@
if (!targetedPreview)
targetedPreview = createFallbackTargetedPreview(self, self.containerForContextMenuHintPreviews, _positionInformation.bounds);
- if (_positionInformation.containerScrollingNodeID) {
- UIScrollView *positionTrackingView = self.webView.scrollView;
- if (auto* scrollingCoordinator = _page->scrollingCoordinatorProxy())
- positionTrackingView = scrollingCoordinator->scrollViewForScrollingNodeID(_positionInformation.containerScrollingNodeID);
+ [self overridePositionTrackingViewForTargetedPreviewIfNecessary:targetedPreview.get() containerScrollingNodeID:_positionInformation.containerScrollingNodeID];
- if ([targetedPreview respondsToSelector:@selector(_setOverridePositionTrackingView:)])
- [targetedPreview _setOverridePositionTrackingView:positionTrackingView];
- }
-
_contextMenuInteractionTargetedPreview = WTFMove(targetedPreview);
return _contextMenuInteractionTargetedPreview.get();
}
Modified: trunk/Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm (275053 => 275054)
--- trunk/Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm 2021-03-25 20:20:47 UTC (rev 275053)
+++ trunk/Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm 2021-03-25 20:22:10 UTC (rev 275054)
@@ -152,7 +152,7 @@
- (UITargetedPreview *)contextMenuInteraction:(UIContextMenuInteraction *)interaction previewForHighlightingMenuWithConfiguration:(UIContextMenuConfiguration *)configuration
{
- return [_view _createTargetedContextMenuHintPreviewIfPossible];
+ return [_view _createTargetedContextMenuHintPreviewForFocusedElement];
}
- (_UIContextMenuStyle *)_contextMenuInteraction:(UIContextMenuInteraction *)interaction styleForMenuWithConfiguration:(UIContextMenuConfiguration *)configuration
@@ -413,14 +413,10 @@
_initialValue = _view.focusedElementInformation.value;
_initialValueAsNumber = _view.focusedElementInformation.valueAsNumber;
[self setDateTimePickerToInitialValue];
-
+
#if USE(UICONTEXTMENU)
- WebKit::InteractionInformationRequest positionInformationRequest { WebCore::IntPoint(_view.focusedElementInformation.lastInteractionLocation) };
- [_view doAfterPositionInformationUpdate:^(WebKit::InteractionInformationAtPosition interactionInformation) {
- [self showDateTimePicker];
- } forRequest:positionInformationRequest];
+ [self showDateTimePicker];
#endif
-
}
- (void)setHour:(NSInteger)hour minute:(NSInteger)minute
Modified: trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm (275053 => 275054)
--- trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm 2021-03-25 20:20:47 UTC (rev 275053)
+++ trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm 2021-03-25 20:22:10 UTC (rev 275054)
@@ -505,11 +505,7 @@
#if USE(UICONTEXTMENU)
_selectMenu = [self createMenu];
-
- WebKit::InteractionInformationRequest positionInformationRequest { WebCore::IntPoint(_view.focusedElementInformation.lastInteractionLocation) };
- [_view doAfterPositionInformationUpdate:^(WebKit::InteractionInformationAtPosition interactionInformation) {
- [self showSelectPicker];
- } forRequest:positionInformationRequest];
+ [self showSelectPicker];
#endif
}
@@ -631,7 +627,7 @@
- (UITargetedPreview *)contextMenuInteraction:(UIContextMenuInteraction *)interaction previewForHighlightingMenuWithConfiguration:(UIContextMenuConfiguration *)configuration
{
- return [_view _createTargetedContextMenuHintPreviewIfPossible];
+ return [_view _createTargetedContextMenuHintPreviewForFocusedElement];
}
- (_UIContextMenuStyle *)_contextMenuInteraction:(UIContextMenuInteraction *)interaction styleForMenuWithConfiguration:(UIContextMenuConfiguration *)configuration
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (275053 => 275054)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-03-25 20:20:47 UTC (rev 275053)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-03-25 20:22:10 UTC (rev 275054)
@@ -866,13 +866,6 @@
RefPtr<Frame> newFocusedFrame = m_page->focusController().focusedFrame();
RefPtr<Element> newFocusedElement = newFocusedFrame ? newFocusedFrame->document()->focusedElement() : nullptr;
- // If the focus has not changed, we need to notify the client anyway, since it might be
- // necessary to start assisting the node.
- // If the node has been focused by _javascript_ without user interaction, the
- // keyboard is not on screen.
- if (newFocusedElement && newFocusedElement == oldFocusedElement)
- elementDidRefocus(*newFocusedElement);
-
if (nodeRespondingToClick.document().settings().contentChangeObserverEnabled()) {
auto& document = nodeRespondingToClick.document();
// Dispatch mouseOut to dismiss tooltip content when tapping on the control bar buttons (cc, settings).
@@ -3083,6 +3076,11 @@
renderer->localToContainerPoint(FloatPoint(), nullptr, UseTransforms, &inFixed);
information.insideFixedPosition = inFixed;
information.isRTL = renderer->style().direction() == TextDirection::RTL;
+
+#if ENABLE(ASYNC_SCROLLING)
+ if (auto* scrollingCoordinator = this->scrollingCoordinator())
+ information.containerScrollingNodeID = scrollingCoordinator->scrollableContainerNodeID(*renderer);
+#endif
} else
information.interactionRect = { };
