From 80e4683e9e512aa5dffb26d1ee52ed0287261f79 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Mon, 4 Dec 2023 09:31:23 +0000 Subject: [PATCH] Bug 1715179 - Allow double-tap-to-zoom to zoom in contents inside OOP iframes. r=botond Depends on D186325 Differential Revision: https://phabricator.services.mozilla.com/D186326 --- gfx/layers/apz/src/APZCTreeManager.cpp | 6 +- gfx/layers/apz/src/AsyncPanZoomController.cpp | 16 +- .../helper_doubletap_zoom_oopif-2.html | 142 ++++++++++++++++++ ...elper_doubletap_zoom_oopif_subframe-1.html | 21 +++ ...elper_doubletap_zoom_oopif_subframe-2.html | 21 +++ .../test_group_double_tap_zoom-2.html | 9 +- .../apz/util/ChromeProcessController.cpp | 10 +- gfx/layers/apz/util/DoubleTapToZoom.cpp | 28 ++-- gfx/layers/apz/util/DoubleTapToZoom.h | 5 +- 9 files changed, 220 insertions(+), 38 deletions(-) create mode 100644 gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif-2.html create mode 100644 gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-1.html create mode 100644 gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-2.html diff --git a/gfx/layers/apz/src/APZCTreeManager.cpp b/gfx/layers/apz/src/APZCTreeManager.cpp index f8e0b7b6069b..55604cfda2a6 100644 --- a/gfx/layers/apz/src/APZCTreeManager.cpp +++ b/gfx/layers/apz/src/APZCTreeManager.cpp @@ -2440,8 +2440,12 @@ void APZCTreeManager::ZoomToRect(const ScrollableLayerGuid& aGuid, } return; } + if (apzc) { - apzc->ZoomToRect(aZoomTarget, aFlags); + apzc = FindZoomableApzc(apzc); + if (apzc) { + apzc->ZoomToRect(aZoomTarget, aFlags); + } } } diff --git a/gfx/layers/apz/src/AsyncPanZoomController.cpp b/gfx/layers/apz/src/AsyncPanZoomController.cpp index 24aace73b089..a471dee48526 100644 --- a/gfx/layers/apz/src/AsyncPanZoomController.cpp +++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp @@ -1230,19 +1230,11 @@ nsEventStatus AsyncPanZoomController::HandleGestureEvent( rv = OnSingleTapConfirmed(tapGestureInput); break; case TapGestureInput::TAPGESTURE_DOUBLE: - // This means that double tapping on an oop iframe "works" in that we - // don't try (and fail) to zoom the oop iframe. But it also means it - // is impossible to zoom to some content inside that oop iframe. - // Instead the best we can do is zoom to the oop iframe itself. This - // is consistent with what Chrome and Safari currently do. Allowing - // zooming to content inside an oop iframe would be decently - // complicated and it doesn't seem worth it. Bug 1715179 is on file - // for this. if (!IsRootContent()) { if (APZCTreeManager* treeManagerLocal = GetApzcTreeManager()) { - if (RefPtr root = - treeManagerLocal->FindZoomableApzc(this)) { - rv = root->OnDoubleTap(tapGestureInput); + if (AsyncPanZoomController* apzc = + treeManagerLocal->FindRootApzcFor(GetLayersId())) { + rv = apzc->OnDoubleTap(tapGestureInput); } } break; @@ -3193,7 +3185,7 @@ nsEventStatus AsyncPanZoomController::OnDoubleTap( RefPtr controller = GetGeckoContentController(); if (controller) { - if (ZoomConstraintsAllowDoubleTapZoom() && + if (rootContentApzc->ZoomConstraintsAllowDoubleTapZoom() && (!GetCurrentTouchBlock() || GetCurrentTouchBlock()->TouchActionAllowsDoubleTapZoom())) { if (Maybe geckoScreenPoint = diff --git a/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif-2.html b/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif-2.html new file mode 100644 index 000000000000..74707b0d8a98 --- /dev/null +++ b/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif-2.html @@ -0,0 +1,142 @@ + + + + + + Tests that double tap to zoom in iframe works regardless whether cross-origin or not + + + + + + + + + + + diff --git a/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-1.html b/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-1.html new file mode 100644 index 000000000000..19da59c59403 --- /dev/null +++ b/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-1.html @@ -0,0 +1,21 @@ + + + + + + + +
+ + diff --git a/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-2.html b/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-2.html new file mode 100644 index 000000000000..f2550e952416 --- /dev/null +++ b/gfx/layers/apz/test/mochitest/helper_doubletap_zoom_oopif_subframe-2.html @@ -0,0 +1,21 @@ + + + + + + + +
+ + diff --git a/gfx/layers/apz/test/mochitest/test_group_double_tap_zoom-2.html b/gfx/layers/apz/test/mochitest/test_group_double_tap_zoom-2.html index 6eb99c64fe23..757290938678 100644 --- a/gfx/layers/apz/test/mochitest/test_group_double_tap_zoom-2.html +++ b/gfx/layers/apz/test/mochitest/test_group_double_tap_zoom-2.html @@ -49,6 +49,10 @@ var subtests = [ {"file": "helper_doubletap_zoom_noscroll.html", "prefs": doubletap_prefs}, {"file": "helper_doubletap_zoom_square.html", "prefs": doubletap_prefs}, {"file": "helper_doubletap_zoom_oopif.html", "prefs": doubletap_prefs}, + {"file": "helper_doubletap_zoom_oopif-2.html", "prefs": [ + ...meta_viewport_and_doubletap_prefs, + ["layout.scroll.disable-pixel-alignment", true], + ]}, {"file": "helper_disallow_doubletap_zoom_inside_oopif.html", "prefs": meta_viewport_and_doubletap_prefs}, {"file": "helper_doubletap_zoom_nothing_listener.html", "prefs": doubletap_prefs}, {"file": "helper_doubletap_zoom_touch_action_manipulation.html", "prefs": meta_viewport_and_doubletap_prefs}, @@ -69,8 +73,11 @@ if (getPlatform() == "mac") { {"file": "helper_doubletap_zoom_noscroll.html?touchpad", "prefs": doubletap_prefs}, {"file": "helper_doubletap_zoom_square.html?touchpad", "prefs": doubletap_prefs}, {"file": "helper_doubletap_zoom_oopif.html?touchpad", "prefs": doubletap_prefs}, + {"file": "helper_doubletap_zoom_oopif-2.html?touchpad", "prefs": doubletap_prefs}, {"file": "helper_disallow_doubletap_zoom_inside_oopif.html?touchpad", "prefs": meta_viewport_and_doubletap_prefs}, - {"file": "helper_doubletap_zoom_nothing_listener.html?touchpad", "prefs": doubletap_prefs}, + {"file": "helper_doubletap_zoom_nothing_listener.html?touchpad", "prefs": [ + ...doubletap_prefs, + ["layout.scroll.disable-pixel-alignment", true]]}, {"file": "helper_doubletap_zoom_touch_action_manipulation.html?touchpad", "prefs": doubletap_prefs}, {"file": "helper_doubletap_zoom_touch_action_manipulation_in_iframe.html?touchpad", "prefs": doubletap_prefs}, ); diff --git a/gfx/layers/apz/util/ChromeProcessController.cpp b/gfx/layers/apz/util/ChromeProcessController.cpp index 5805d42fb80a..a836f0ecdc14 100644 --- a/gfx/layers/apz/util/ChromeProcessController.cpp +++ b/gfx/layers/apz/util/ChromeProcessController.cpp @@ -146,14 +146,8 @@ void ChromeProcessController::HandleDoubleTap( ZoomTarget zoomTarget = CalculateRectToZoomTo(document, aPoint, aDoubleTapToZoomMetrics); - uint32_t presShellId; - ScrollableLayerGuid::ViewID viewId; - if (APZCCallbackHelper::GetOrCreateScrollIdentifiers( - document->GetDocumentElement(), &presShellId, &viewId)) { - mAPZCTreeManager->ZoomToRect( - ScrollableLayerGuid(aGuid.mLayersId, presShellId, viewId), zoomTarget, - ZoomToRectBehavior::DEFAULT_BEHAVIOR); - } + mAPZCTreeManager->ZoomToRect(aGuid, zoomTarget, + ZoomToRectBehavior::DEFAULT_BEHAVIOR); } void ChromeProcessController::HandleTap( diff --git a/gfx/layers/apz/util/DoubleTapToZoom.cpp b/gfx/layers/apz/util/DoubleTapToZoom.cpp index 13987adeb78d..23206bfd9c7e 100644 --- a/gfx/layers/apz/util/DoubleTapToZoom.cpp +++ b/gfx/layers/apz/util/DoubleTapToZoom.cpp @@ -100,11 +100,11 @@ static dom::Element* GetNearbyTableCell( // level content document coordinates. static CSSRect GetBoundingContentRect( const dom::Element* aElement, - const RefPtr& aRootContentDocument, + const RefPtr& aInProcessRootContentDocument, const nsIScrollableFrame* aRootScrollFrame, const DoubleTapToZoomMetrics& aMetrics, mozilla::Maybe* aOutNearestScrollClip = nullptr) { - if (aRootContentDocument->IsTopLevelContentDocument()) { + if (aInProcessRootContentDocument->IsTopLevelContentDocument()) { return nsLayoutUtils::GetBoundingContentRect(aElement, aRootScrollFrame, aOutNearestScrollClip); } @@ -124,7 +124,7 @@ static CSSRect GetBoundingContentRect( static bool ShouldZoomToElement( const nsCOMPtr& aElement, - const RefPtr& aRootContentDocument, + const RefPtr& aInProcessRootContentDocument, nsIScrollableFrame* aRootScrollFrame, const DoubleTapToZoomMetrics& aMetrics) { if (nsIFrame* frame = aElement->GetPrimaryFrame()) { @@ -139,7 +139,7 @@ static bool ShouldZoomToElement( // Trying to zoom to the html element will just end up scrolling to the start // of the document, return false and we'll run out of elements and just // zoomout (without scrolling to the start). - if (aElement->OwnerDoc() == aRootContentDocument && + if (aElement->OwnerDoc() == aInProcessRootContentDocument && aElement->IsHTMLElement(nsGkAtoms::html)) { return false; } @@ -153,8 +153,8 @@ static bool ShouldZoomToElement( // don't want to zoom to them. This heuristic is quite naive and leaves a lot // to be desired. if (dom::Element* tableCell = GetNearbyTableCell(aElement)) { - CSSRect rect = GetBoundingContentRect(tableCell, aRootContentDocument, - aRootScrollFrame, aMetrics); + CSSRect rect = GetBoundingContentRect( + tableCell, aInProcessRootContentDocument, aRootScrollFrame, aMetrics); if (rect.width < 0.3 * aMetrics.mRootScrollableRect.width) { return false; } @@ -235,15 +235,15 @@ static bool HasNonPassiveWheelListenerOnAncestor(nsIContent* aContent) { } ZoomTarget CalculateRectToZoomTo( - const RefPtr& aRootContentDocument, const CSSPoint& aPoint, - const DoubleTapToZoomMetrics& aMetrics) { + const RefPtr& aInProcessRootContentDocument, + const CSSPoint& aPoint, const DoubleTapToZoomMetrics& aMetrics) { // Ensure the layout information we get is up-to-date. - aRootContentDocument->FlushPendingNotifications(FlushType::Layout); + aInProcessRootContentDocument->FlushPendingNotifications(FlushType::Layout); // An empty rect as return value is interpreted as "zoom out". const CSSRect zoomOut; - RefPtr presShell = aRootContentDocument->GetPresShell(); + RefPtr presShell = aInProcessRootContentDocument->GetPresShell(); if (!presShell) { return ZoomTarget{zoomOut, CantZoomOutBehavior::ZoomIn}; } @@ -255,7 +255,7 @@ ZoomTarget CalculateRectToZoomTo( } CSSPoint documentRelativePoint = - aRootContentDocument->IsTopLevelContentDocument() + aInProcessRootContentDocument->IsTopLevelContentDocument() ? CSSPoint::FromAppUnits(ViewportUtils::VisualToLayout( CSSPoint::ToAppUnits(aPoint), presShell)) + CSSPoint::FromAppUnits(rootScrollFrame->GetScrollPosition()) @@ -272,7 +272,7 @@ ZoomTarget CalculateRectToZoomTo( ? CantZoomOutBehavior::Nothing : CantZoomOutBehavior::ZoomIn; - while (element && !ShouldZoomToElement(element, aRootContentDocument, + while (element && !ShouldZoomToElement(element, aInProcessRootContentDocument, rootScrollFrame, aMetrics)) { element = element->GetFlattenedTreeParentElement(); } @@ -284,8 +284,8 @@ ZoomTarget CalculateRectToZoomTo( Maybe nearestScrollClip; CSSRect rect = - GetBoundingContentRect(element, aRootContentDocument, rootScrollFrame, - aMetrics, &nearestScrollClip); + GetBoundingContentRect(element, aInProcessRootContentDocument, + rootScrollFrame, aMetrics, &nearestScrollClip); // In some cases, like overflow: visible and overflowing content, the bounding // client rect of the targeted element won't contain the point the user double diff --git a/gfx/layers/apz/util/DoubleTapToZoom.h b/gfx/layers/apz/util/DoubleTapToZoom.h index dbc8dabbf463..3c4503405e09 100644 --- a/gfx/layers/apz/util/DoubleTapToZoom.h +++ b/gfx/layers/apz/util/DoubleTapToZoom.h @@ -72,10 +72,11 @@ struct DoubleTapToZoomMetrics { * For a double tap at |aPoint|, return a ZoomTarget struct with contains a rect * to which the browser should zoom in response (see ZoomTarget definition for * more details). An empty rect means the browser should zoom out. |aDocument| - * should be the root content document for the content that was tapped. + * should be the in-process root content document for the content that was + * tapped. */ ZoomTarget CalculateRectToZoomTo( - const RefPtr& aRootContentDocument, + const RefPtr& aInProcessRootContentDocument, const CSSPoint& aPoint, const DoubleTapToZoomMetrics& aMetrics); } // namespace layers