[webkit-changes] [WebKit/WebKit] d7b5c0: REGRESSION (285237 at main): khanacademy.org: Live Te...

[Wenson Hsieh]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: d7b5c032766f7771a5fa80cdca4966c3d4b56539
      https://github.com/WebKit/WebKit/commit/d7b5c032766f7771a5fa80cdca4966c3d4b56539
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2025-01-15 (Wed, 15 Jan 2025)

  Changed paths:
    M Source/WebCore/rendering/RenderVideo.cpp
    M Source/WebCore/rendering/RenderVideo.h
    M Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
    M Source/WebKit/WebProcess/WebPage/WebPage.cpp
    M Source/WebKit/WebProcess/cocoa/VideoPresentationManager.mm

  Log Message:
  -----------
  REGRESSION (285237 at main): khanacademy.org: Live Text selection is misplaced in fullscreen video on iPad
https://bugs.webkit.org/show_bug.cgi?id=286007
rdar://142822957

Reviewed by Abrar Rahman Protyasha and Megan Gardner.

The prior fix in 285237 at main is incorrect, since the video on New York Times wasn't actually offset
by the top obscured inset. Rather, the root cause is that the interaction bounds we're using to
position the `VKCImageAnalysisInteraction` simply uses `RenderVideo::videoBox()`, which doesn't
include the absolute left/top offset of the renderer itself. As a result, some videos just happen to
be positioned more accurately after the changes in 285237 at main, while others (for which the renderer
is already at the origin) become offset.

`VideoPresentationManager` already has a helper, `inlineVideoFrame`, which handles this computation
correctly — to fix this, we simply pull that logic out into a separate `RenderVideo` helper method,
and use it in both places.

This isn't currently testable in API or layout tests — I manually verified that Live Text selections
are positioned correctly across various video players (youtube.com, YouTube embeds, websites from
previous bug reports like New York Times, and simple test cases).

* Source/WebCore/rendering/RenderVideo.cpp:
(WebCore::RenderVideo::videoBoxInRootView const):

Pull logic to compute the video box in root view coordinates into a separate helper method, which we
can use in both `VideoPresentationManager` and `WebPage` (when computing bounds for Live Text).

* Source/WebCore/rendering/RenderVideo.h:
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView contentsRectForImageAnalysisInteraction:]):

Revert the previous (incorrect) attempt to fix this bug.

* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::beginTextRecognitionForVideoInElementFullScreen):

Use `videoBoxInRootView()` (see above).

* Source/WebKit/WebProcess/cocoa/VideoPresentationManager.mm:
(WebKit::inlineVideoFrame):

Move some of this logic into the new `videoBoxInRootView()` method above.

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



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