[webkit-changes] [WebKit/WebKit] 4aac13: REGRESSION (285925 at main): [iOS] Selection highligh...

[Wenson Hsieh]

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

  Changed paths:
    A LayoutTests/editing/selection/ios/do-not-hide-selection-in-non-editable-visible-container-expected.txt
    A LayoutTests/editing/selection/ios/do-not-hide-selection-in-non-editable-visible-container.html
    M Source/WebCore/rendering/RenderObject.cpp
    M Source/WebCore/rendering/RenderObject.h
    M Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm
    M Source/WebKit/WebProcess/WebPage/WebPage.h

  Log Message:
  -----------
  REGRESSION (285925 at main): [iOS] Selection highlights for non-editable content sometimes disappear
https://bugs.webkit.org/show_bug.cgi?id=286490
rdar://143296175

Reviewed by Abrar Rahman Protyasha.

After the changes in https://commits.webkit.org/285925@main, we now apply the "hidden selection
container" heuristic to non-editable, non-collapsed selection ranges as well as editable selections
(e.g. Google Docs). This worked by first finding the common ancestor `Node` that contains both
selection endpoints, and then checking if that container's renderer is either in a completely- or
nearly-transparent layer, or is otherwise completely clipped out (based on visible repaint rects).

However, this logic breaks in the case where the selection endpoints' common ancestor in the render
tree is _not_ the same as the common ancestor in the DOM — for example, when using the inline `font`
element like so:

```
<font face="…">
    <p>abc</p> <!-- selection starts here -->
    …
    <p>def</p> <!-- selection ends here -->
</font>
```

In this case, the `font` element is common ancestor in the DOM, but an anonymous block renderer is
the common ancestor in the actual render tree. This causes us to incorrectly treat this selection as
fully clipped, based on the fact that the inline `font` element renderer is empty. To fix this, we
adjust the heuristic to find the common ancestor in the render tree that contains both selection
endpoints, and then check if that common ancestor is hidden.

* LayoutTests/editing/selection/ios/do-not-hide-selection-in-non-editable-visible-container-expected.txt: Added.
* LayoutTests/editing/selection/ios/do-not-hide-selection-in-non-editable-visible-container.html: Added.

Add a new layout test to exercise this fix.

* Source/WebCore/rendering/RenderObject.cpp:
(WebCore::RenderObject::hasEmptyVisibleRectRespectingParentFrames const):
(WebCore::RenderObject::hasNonEmptyVisibleRectRespectingParentFrames const): Deleted.

Drive-by fix: remove the `Non` from this method, which returns true if and only if the renderer's
(or one of its parent frames') visible rect is empty. Note that the only call site expects this
behavior, so the resulting behavior is correct (the method name is just misleading).

* Source/WebCore/rendering/RenderObject.h:
* Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:
(WebKit::WebPage::isTransparentOrFullyClipped const):
(WebKit::closestCommonContainerInRenderTree):

Add a helper method to return the deepest common renderer that contains both selection endpoints.

(WebKit::WebPage::getPlatformEditorStateCommon const):
* Source/WebKit/WebProcess/WebPage/WebPage.h:

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



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