Modified: branches/safari-608-branch/Source/WebCore/ChangeLog (251336 => 251337)
--- branches/safari-608-branch/Source/WebCore/ChangeLog 2019-10-20 17:32:45 UTC (rev 251336)
+++ branches/safari-608-branch/Source/WebCore/ChangeLog 2019-10-20 17:32:48 UTC (rev 251337)
@@ -1,5 +1,92 @@
2019-10-15 Kocsen Chung <kocsen_ch...@apple.com>
+ Cherry-pick r250642. rdar://problem/56280704
+
+ [iOS] When hit testing for a context menu interaction, do not consider whether the element is contenteditable
+ https://bugs.webkit.org/show_bug.cgi?id=202498
+ <rdar://problem/54723131>
+
+ Reviewed by Tim Horton.
+
+ Source/WebCore:
+
+ When the user selects a context menu action, WebKit performs a hit test in order to find the
+ acted-on element on the page. This is separate from the hit test performed to generate the
+ context menu's targeted preview. Since an arbitrary amount of time can elapse between
+ preview generation and action selection, this second hit-tests might return a different
+ element.
+
+ One case where we know a different element can be returned is in apps that dynamically
+ enable and disable editing. If editing is disabled when the first hit test occurs but is
+ enabled when the second one occurs, different elements will be returned due to
+ Frame::qualifyingNodeAtViewportLocation preferring to return the root editable element when
+ the approximate node is contenteditable.
+
+ While the appropriate long-term fix is to only hit-test once and use that element for both
+ preview generation and action selection, this patch implements a short-term fix to address
+ the specific problem in rdar://problem/54723131 by disabling the contenteditable behavior
+ described above for context menu interaction hit testing.
+
+ The long-term fix is tracked by <https://webkit.org/b/202499>.
+
+ * page/Frame.h:
+ * page/ios/FrameIOS.mm:
+ (WebCore::Frame::qualifyingNodeAtViewportLocation):
+ (WebCore::Frame::approximateNodeAtViewportLocationLegacy):
+ (WebCore::ancestorRespondingToClickEventsNodeQualifier):
+ (WebCore::Frame::nodeRespondingToClickEvents):
+ (WebCore::Frame::nodeRespondingToDoubleClickEvent):
+ (WebCore::Frame::nodeRespondingToInteraction):
+ (WebCore::Frame::nodeRespondingToScrollWheelEvents):
+
+ Source/WebKit:
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::startInteractionWithElementAtPosition): Changed to call
+ WebCore::Frame::nodeRespondingToInteraction.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250642 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-10-02 Andy Estes <aes...@apple.com>
+
+ [iOS] When hit testing for a context menu interaction, do not consider whether the element is contenteditable
+ https://bugs.webkit.org/show_bug.cgi?id=202498
+ <rdar://problem/54723131>
+
+ Reviewed by Tim Horton.
+
+ When the user selects a context menu action, WebKit performs a hit test in order to find the
+ acted-on element on the page. This is separate from the hit test performed to generate the
+ context menu's targeted preview. Since an arbitrary amount of time can elapse between
+ preview generation and action selection, this second hit-tests might return a different
+ element.
+
+ One case where we know a different element can be returned is in apps that dynamically
+ enable and disable editing. If editing is disabled when the first hit test occurs but is
+ enabled when the second one occurs, different elements will be returned due to
+ Frame::qualifyingNodeAtViewportLocation preferring to return the root editable element when
+ the approximate node is contenteditable.
+
+ While the appropriate long-term fix is to only hit-test once and use that element for both
+ preview generation and action selection, this patch implements a short-term fix to address
+ the specific problem in rdar://problem/54723131 by disabling the contenteditable behavior
+ described above for context menu interaction hit testing.
+
+ The long-term fix is tracked by <https://webkit.org/b/202499>.
+
+ * page/Frame.h:
+ * page/ios/FrameIOS.mm:
+ (WebCore::Frame::qualifyingNodeAtViewportLocation):
+ (WebCore::Frame::approximateNodeAtViewportLocationLegacy):
+ (WebCore::ancestorRespondingToClickEventsNodeQualifier):
+ (WebCore::Frame::nodeRespondingToClickEvents):
+ (WebCore::Frame::nodeRespondingToDoubleClickEvent):
+ (WebCore::Frame::nodeRespondingToInteraction):
+ (WebCore::Frame::nodeRespondingToScrollWheelEvents):
+
+2019-10-15 Kocsen Chung <kocsen_ch...@apple.com>
+
Cherry-pick r249517. rdar://problem/56000099
Mail appears to be double inverting code copied from Notes, Xcode, or Terminal.
Modified: branches/safari-608-branch/Source/WebCore/page/Frame.h (251336 => 251337)
--- branches/safari-608-branch/Source/WebCore/page/Frame.h 2019-10-20 17:32:45 UTC (rev 251336)
+++ branches/safari-608-branch/Source/WebCore/page/Frame.h 2019-10-20 17:32:48 UTC (rev 251337)
@@ -221,6 +221,7 @@
WEBCORE_EXPORT Node* deepestNodeAtLocation(const FloatPoint& viewportLocation);
WEBCORE_EXPORT Node* nodeRespondingToClickEvents(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation, SecurityOrigin* = nullptr);
WEBCORE_EXPORT Node* nodeRespondingToDoubleClickEvent(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation);
+ WEBCORE_EXPORT Node* nodeRespondingToInteraction(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation);
WEBCORE_EXPORT Node* nodeRespondingToScrollWheelEvents(const FloatPoint& viewportLocation);
WEBCORE_EXPORT Node* approximateNodeAtViewportLocationLegacy(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation);
@@ -334,7 +335,10 @@
#if PLATFORM(IOS_FAMILY)
void betterApproximateNode(const IntPoint& testPoint, const NodeQualifier&, Node*& best, Node* failedNode, IntPoint& bestPoint, IntRect& bestRect, const IntRect& testRect);
bool hitTestResultAtViewportLocation(const FloatPoint& viewportLocation, HitTestResult&, IntPoint& center);
- Node* qualifyingNodeAtViewportLocation(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation, const NodeQualifier&, bool shouldApproximate);
+
+ enum class ShouldApproximate : bool { No, Yes };
+ enum class ShouldFindRootEditableElement : bool { No, Yes };
+ Node* qualifyingNodeAtViewportLocation(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation, const NodeQualifier&, ShouldApproximate, ShouldFindRootEditableElement = ShouldFindRootEditableElement::Yes);
void setTimersPausedInternal(bool);
Modified: branches/safari-608-branch/Source/WebCore/page/ios/FrameIOS.mm (251336 => 251337)
--- branches/safari-608-branch/Source/WebCore/page/ios/FrameIOS.mm 2019-10-20 17:32:45 UTC (rev 251336)
+++ branches/safari-608-branch/Source/WebCore/page/ios/FrameIOS.mm 2019-10-20 17:32:48 UTC (rev 251337)
@@ -266,8 +266,12 @@
return true;
}
-Node* Frame::qualifyingNodeAtViewportLocation(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation, const NodeQualifier& nodeQualifierFunction, bool shouldApproximate)
+Node* Frame::qualifyingNodeAtViewportLocation(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation, const NodeQualifier& nodeQualifierFunction, ShouldApproximate shouldApproximate, ShouldFindRootEditableElement shouldFindRootEditableElement)
{
+#if !USE(UIKIT_EDITING)
+ UNUSED_PARAM(shouldFindRootEditableElement);
+#endif
+
adjustedViewportLocation = viewportLocation;
IntPoint testCenter;
@@ -282,12 +286,12 @@
Node* approximateNode = nodeQualifierFunction(candidateInfo, 0, 0);
#if USE(UIKIT_EDITING)
- if (approximateNode && approximateNode->isContentEditable()) {
+ if (shouldFindRootEditableElement == ShouldFindRootEditableElement::Yes && approximateNode && approximateNode->isContentEditable()) {
// If we are in editable content, we look for the root editable element.
approximateNode = approximateNode->rootEditableElement();
// If we have a focusable node, there is no need to approximate.
if (approximateNode)
- shouldApproximate = false;
+ shouldApproximate = ShouldApproximate::No;
}
#endif
@@ -298,7 +302,7 @@
static const float unscaledSearchRadius = 15;
int searchRadius = static_cast<int>(unscaledSearchRadius * ppiFactor / scale);
- if (approximateNode && shouldApproximate) {
+ if (approximateNode && shouldApproximate == ShouldApproximate::Yes) {
const float testOffsets[] = {
-.3f, -.3f,
-.6f, -.6f,
@@ -319,7 +323,7 @@
break;
}
}
- } else if (!approximateNode && shouldApproximate) {
+ } else if (!approximateNode && shouldApproximate == ShouldApproximate::Yes) {
// Grab the closest parent element of our failed candidate node.
Node* candidate = candidateInfo.innerNode();
Node* failedNode = candidate;
@@ -367,7 +371,7 @@
IntPoint p = m_view->contentsToWindow(bestPoint);
adjustedViewportLocation = p;
#if USE(UIKIT_EDITING)
- if (approximateNode->isContentEditable()) {
+ if (shouldFindRootEditableElement == ShouldFindRootEditableElement::Yes && approximateNode->isContentEditable()) {
// When in editable content, look for the root editable node again,
// since this could be the node found with the approximation.
approximateNode = approximateNode->rootEditableElement();
@@ -448,12 +452,12 @@
return nullptr;
};
- return qualifyingNodeAtViewportLocation(viewportLocation, adjustedViewportLocation, WTFMove(ancestorRespondingToClickEvents), true);
+ return qualifyingNodeAtViewportLocation(viewportLocation, adjustedViewportLocation, WTFMove(ancestorRespondingToClickEvents), ShouldApproximate::Yes);
}
-Node* Frame::nodeRespondingToClickEvents(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation, SecurityOrigin* securityOrigin)
+static inline NodeQualifier ancestorRespondingToClickEventsNodeQualifier(SecurityOrigin* securityOrigin = nullptr)
{
- auto&& ancestorRespondingToClickEvents = [securityOrigin](const HitTestResult& hitTestResult, Node* terminationNode, IntRect* nodeBounds) -> Node* {
+ return [securityOrigin](const HitTestResult& hitTestResult, Node* terminationNode, IntRect* nodeBounds) -> Node* {
if (nodeBounds)
*nodeBounds = IntRect();
@@ -478,8 +482,11 @@
return nullptr;
};
+}
- return qualifyingNodeAtViewportLocation(viewportLocation, adjustedViewportLocation, WTFMove(ancestorRespondingToClickEvents), true);
+Node* Frame::nodeRespondingToClickEvents(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation, SecurityOrigin* securityOrigin)
+{
+ return qualifyingNodeAtViewportLocation(viewportLocation, adjustedViewportLocation, ancestorRespondingToClickEventsNodeQualifier(securityOrigin), ShouldApproximate::Yes);
}
Node* Frame::nodeRespondingToDoubleClickEvent(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation)
@@ -506,9 +513,14 @@
return nullptr;
};
- return qualifyingNodeAtViewportLocation(viewportLocation, adjustedViewportLocation, WTFMove(ancestorRespondingToDoubleClickEvent), true);
+ return qualifyingNodeAtViewportLocation(viewportLocation, adjustedViewportLocation, WTFMove(ancestorRespondingToDoubleClickEvent), ShouldApproximate::Yes);
}
+Node* Frame::nodeRespondingToInteraction(const FloatPoint& viewportLocation, FloatPoint& adjustedViewportLocation)
+{
+ return qualifyingNodeAtViewportLocation(viewportLocation, adjustedViewportLocation, ancestorRespondingToClickEventsNodeQualifier(), ShouldApproximate::Yes, ShouldFindRootEditableElement::No);
+}
+
Node* Frame::nodeRespondingToScrollWheelEvents(const FloatPoint& viewportLocation)
{
auto&& ancestorRespondingToScrollWheelEvents = [](const HitTestResult& hitTestResult, Node* terminationNode, IntRect* nodeBounds) -> Node* {
@@ -539,7 +551,7 @@
};
FloatPoint adjustedViewportLocation;
- return qualifyingNodeAtViewportLocation(viewportLocation, adjustedViewportLocation, WTFMove(ancestorRespondingToScrollWheelEvents), false);
+ return qualifyingNodeAtViewportLocation(viewportLocation, adjustedViewportLocation, WTFMove(ancestorRespondingToScrollWheelEvents), ShouldApproximate::No);
}
int Frame::preferredHeight() const
Modified: branches/safari-608-branch/Source/WebKit/ChangeLog (251336 => 251337)
--- branches/safari-608-branch/Source/WebKit/ChangeLog 2019-10-20 17:32:45 UTC (rev 251336)
+++ branches/safari-608-branch/Source/WebKit/ChangeLog 2019-10-20 17:32:48 UTC (rev 251337)
@@ -1,5 +1,67 @@
2019-10-15 Kocsen Chung <kocsen_ch...@apple.com>
+ Cherry-pick r250642. rdar://problem/56280704
+
+ [iOS] When hit testing for a context menu interaction, do not consider whether the element is contenteditable
+ https://bugs.webkit.org/show_bug.cgi?id=202498
+ <rdar://problem/54723131>
+
+ Reviewed by Tim Horton.
+
+ Source/WebCore:
+
+ When the user selects a context menu action, WebKit performs a hit test in order to find the
+ acted-on element on the page. This is separate from the hit test performed to generate the
+ context menu's targeted preview. Since an arbitrary amount of time can elapse between
+ preview generation and action selection, this second hit-tests might return a different
+ element.
+
+ One case where we know a different element can be returned is in apps that dynamically
+ enable and disable editing. If editing is disabled when the first hit test occurs but is
+ enabled when the second one occurs, different elements will be returned due to
+ Frame::qualifyingNodeAtViewportLocation preferring to return the root editable element when
+ the approximate node is contenteditable.
+
+ While the appropriate long-term fix is to only hit-test once and use that element for both
+ preview generation and action selection, this patch implements a short-term fix to address
+ the specific problem in rdar://problem/54723131 by disabling the contenteditable behavior
+ described above for context menu interaction hit testing.
+
+ The long-term fix is tracked by <https://webkit.org/b/202499>.
+
+ * page/Frame.h:
+ * page/ios/FrameIOS.mm:
+ (WebCore::Frame::qualifyingNodeAtViewportLocation):
+ (WebCore::Frame::approximateNodeAtViewportLocationLegacy):
+ (WebCore::ancestorRespondingToClickEventsNodeQualifier):
+ (WebCore::Frame::nodeRespondingToClickEvents):
+ (WebCore::Frame::nodeRespondingToDoubleClickEvent):
+ (WebCore::Frame::nodeRespondingToInteraction):
+ (WebCore::Frame::nodeRespondingToScrollWheelEvents):
+
+ Source/WebKit:
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::startInteractionWithElementAtPosition): Changed to call
+ WebCore::Frame::nodeRespondingToInteraction.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250642 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-10-02 Andy Estes <aes...@apple.com>
+
+ [iOS] When hit testing for a context menu interaction, do not consider whether the element is contenteditable
+ https://bugs.webkit.org/show_bug.cgi?id=202498
+ <rdar://problem/54723131>
+
+ Reviewed by Tim Horton.
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::startInteractionWithElementAtPosition): Changed to call
+ WebCore::Frame::nodeRespondingToInteraction.
+
+2019-10-15 Kocsen Chung <kocsen_ch...@apple.com>
+
Cherry-pick r250431. rdar://problem/55927251
Storage Access API: document.hasStorageAccess() should return false by default
Modified: branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (251336 => 251337)
--- branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-10-20 17:32:45 UTC (rev 251336)
+++ branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-10-20 17:32:48 UTC (rev 251337)
@@ -2857,8 +2857,14 @@
void WebPage::startInteractionWithElementAtPosition(const WebCore::IntPoint& point)
{
+ // FIXME: We've already performed a hit test when the long-press gesture was recognized and we
+ // used that result to generate the targeted preview, but now we are hit testing again in order
+ // to perform the selected action. Since an arbitrary amount of time can elapse between
+ // generating the targeted preview and the user selecting an action, it's possible that this
+ // second hit test will find a different element than the first one, leading to bugs like
+ // <rdar://problem/54723131>. We should re-use the results of the first hit test instead.
FloatPoint adjustedPoint;
- m_interactionNode = m_page->mainFrame().nodeRespondingToClickEvents(point, adjustedPoint);
+ m_interactionNode = m_page->mainFrame().nodeRespondingToInteraction(point, adjustedPoint);
}
void WebPage::stopInteraction()
