[webkit-changes] [WebKit/WebKit] 4923f3: [UnifiedPDF] At certain scales, "Previous Page" co...

[Abrar Rahman Protyasha]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 4923f3abc3d510db9ae1f74812b75c1debb62157
      https://github.com/WebKit/WebKit/commit/4923f3abc3d510db9ae1f74812b75c1debb62157
  Author: Abrar Rahman Protyasha <a_protyasha at apple.com>
  Date:   2025-01-24 (Fri, 24 Jan 2025)

  Changed paths:
    M Source/WebCore/platform/ScrollableArea.cpp
    M Source/WebCore/platform/ScrollableArea.h
    M Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFDiscretePresentationController.h
    M Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFDiscretePresentationController.mm
    M Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFPresentationController.h
    M Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFPresentationController.mm
    M Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFScrollingPresentationController.h
    M Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFScrollingPresentationController.mm
    M Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h
    M Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm

  Log Message:
  -----------
  [UnifiedPDF] At certain scales, "Previous Page" context menu option does not navigate to previous page in 2-up continuous mode
https://bugs.webkit.org/show_bug.cgi?id=286440
rdar://139817364

Reviewed by Simon Fraser and Megan Gardner.

The "Previous Page" context menu option would feel like a no-op (or
even, would navigate forwards) at certain scales because we would get
the index of the page "currently" in view, i.e. center-most page, and
only navigate backwards by a row. When zoomed out far enough, this
center-most page and the pages offset by a row are all roughly in the
middle of the page, so we don't end up invoking revealPage() on the
appropriate index.

This patch tries to be smarter about which page we land on with the
context menu option. We ideally want to end up on the first page above
the root view bounds, but this does not translate well to discrete
display modes because you only ever have one row visible in the root
view. Thus, we either go one row behind the page "currently" in view, or
use the metric I just mentioned.

Moreover, when zoomed out far enough, if the top edge of a page
perfectly aligns with the root view's edge, then our new metric makes us
land on said page consistently, taking us back to no-op-like navigation.
To avoid this, we teach the plugin to consult the return value of the
ScrollableArea helpers it uses under revealPage(). If we don't
successfully scroll during the reveal operation, we move back one row
and try again. More details about this later in the commit message.

* Source/WebCore/platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::scrollToPositionWithoutAnimation):
* Source/WebCore/platform/ScrollableArea.h:

Forward ScrollAnimator::scrollToPositionWithoutAnimation()'s return value
through its ScrollableArea caller, which the plugin will use to
determine the success of scrollToPointInContentsSpace() calls.

* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFDiscretePresentationController.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFDiscretePresentationController.mm:
(WebKit::PDFDiscretePresentationController::pageIndexForCurrentView const):
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFPresentationController.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFPresentationController.mm:
(WebKit::PDFPresentationController::pdfPositionForCurrentView const):
(WebKit::PDFPresentationController::anchorPointInDocumentSpace const):
(WebKit::PDFPresentationController::anchorPointForDocument const): Deleted.
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFScrollingPresentationController.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/PDFScrollingPresentationController.mm:
(WebKit::PDFScrollingPresentationController::pageIndexForCurrentView const):
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm:
(WebKit::UnifiedPDFPlugin::updateLayout):

The changes above are all a drive-by naming change to better reflect the
coordinate space for `anchorPoint`.

(WebKit::UnifiedPDFPlugin::revealPage):
(WebKit::UnifiedPDFPlugin::scrollToPointInContentsSpace):

Return the result of ScrollableArea::scrollToPositionWithoutAnimation()
for the continuous display mode case. For discrete display modes, we can
always return true under the assumption that callers have ensured that
the correct page is visible.

(WebKit::UnifiedPDFPlugin::performContextMenuAction):

Canonical link: https://commits.webkit.org/289337@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications