[webkit-changes] [WebKit/WebKit] 93a8eb: [Writing Tools] Affordance doesn't show up when ho...

[Aditya Keerthi]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 93a8eb15de1e5ec7797e38c9b601ec1a58546019
      https://github.com/WebKit/WebKit/commit/93a8eb15de1e5ec7797e38c9b601ec1a58546019
  Author: Aditya Keerthi <akeerthi at apple.com>
  Date:   2024-08-15 (Thu, 15 Aug 2024)

  Changed paths:
    M Source/WebKit/WebProcess/WebPage/WebPage.cpp
    M Tools/TestWebKitAPI/Tests/mac/WKWebViewMacEditingTests.mm

  Log Message:
  -----------
  [Writing Tools] Affordance doesn't show up when hovering over multiple lines of text containing newlines
https://bugs.webkit.org/show_bug.cgi?id=278186
rdar://129721335

Reviewed by Wenson Hsieh.

Writing Tools determines whether to show an affordance when hovering over the
current selection using `-[NSTextInputClient_Async firstRectForCharacterRange:completionHandler:]`
to compute the number of lines. If the number of lines in greater than a defined
threshold, the affordance is displayed.

The idea behind using `firstRectForCharacterRange` to compute the number of lines
is to iteratively call the method, using the returned `actualRange` to keep track
of the remaining "unprocessed" range. However, this approach is currently breaking
down, as Writing Tools is observing that an `actualRange` with zero length ends up
getting returned when a newline is encountered.

However, the underlying issue is that WebKit's computation of `actualRange` is
currently incorrect. When a line ends with a newline, the newline should be
included in the length of the range. Currently, it is not, as range determination
is simply done using `endOfLine`, and newlines are only included when going to
the start of the next line. This discrepency results in Writing Tools starting to
request incorrect ranges, and the wrong information is processed.

Fix by ensuring that the `actualRange` for `firstRectForCharacterRange` includes
newlines for lines that end with one.

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

If `endOfLine` has upstream affinity, then no changes are necessary, as there are
no characters between the line boundary.

However, if the returned value is on the same line, and has downstream affinity,
get the start of the next line using `positionOfNextBoundaryOfGranularity`. This
ensures that the newline character between lines is included in the length of
the returned range.

* Tools/TestWebKitAPI/Tests/mac/WKWebViewMacEditingTests.mm:
(-[WKWebView _selectedRange]):
(TEST(WKWebViewMacEditingTests, FirstRectForCharacterRange)):

Rebaseline to account for that fact that the newline character is included in the
length.

(TEST(WKWebViewMacEditingTests, FirstRectForCharacterRangeWithNewlinesAndWrapping)):

Test that `firstRectForCharacterRange` can be used to count lines for content with
newlines and line wrapping.

(TEST(WKWebViewMacEditingTests, FirstRectForCharacterRangeForPartialLineWithNewlinesAndWrapping)):

Ensure the changes do not break scenarios where rects are requested for the middle
of the line.

(TEST(WKWebViewMacEditingTests, FirstRectForCharacterRangeWithNewlinesAndWrappingLineBreakAfterWhiteSpace)):

Test that `firstRectForCharacterRange` can be used to count lines for content with
newlines, line wrapping, and `line-break: after-white-space`. Importantly, this
tests that going to the start of the next line is not attempted when line wrapping
is performed and the line ends with a space.

(TEST(WKWebViewMacEditingTests, FirstRectForCharacterRangeForPartialLineWithNewlinesAndWrappingLineBreakAfterWhiteSpace)):

Ensure the changes do not break scenarios where rects are requested for the middle

of the line and `line-break: after-white-space` is used.
Canonical link: https://commits.webkit.org/282327@main



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