[webkit-changes] [WebKit/WebKit] b20f02: [iOS] Entering every other character in the find b...

[Aditya Keerthi]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: b20f02feaf02b1c2c44ebfcdadda9dd4652e2cce
      https://github.com/WebKit/WebKit/commit/b20f02feaf02b1c2c44ebfcdadda9dd4652e2cce
  Author: Aditya Keerthi <akeerthi at apple.com>
  Date:   2022-09-16 (Fri, 16 Sep 2022)

  Changed paths:
    M Source/WebCore/editing/Editor.cpp
    M Source/WebCore/page/FrameView.cpp
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm

  Log Message:
  -----------
  [iOS] Entering every other character in the find bar causes HTML notes to scroll to the top
https://bugs.webkit.org/show_bug.cgi?id=245223
rdar://99843009

Reviewed by Megan Gardner and Tim Horton.

250836 at main updated the `TemporarySelectionChange` logic to incorporate the
user-triggered option for synchronous selection updates. 251398 at main adopted
this option to ensure found text is scrolled into view.

However, the incorporation of the user-triggered option in 250836 at main was
incorrect. The user-triggered option was passed directly into
`FrameSelection::defaultSetSelectionOptions`, for both the temporary change, and
the selection restoration. `defaultSetSelectionOptions` adds `RevealSelection`
and `FireSelectEvent` when the user-triggered option is specified.

Passing the option into `defaultSetSelectionOptions` in `TemporarySelectionChange`
is incorrect for multiple reasons:

1. `TemporarySelectionChange` has its own option to control selection revealing.
2. The default options are applied during selection restoration, breaking the use of `TemporarySelectionChange` to scroll to a range.
3. The select event should not be fired for a `TemporarySelectionChange`.

(2) is the cause of this bug, as the `RevealSelection` option is specified once the
original selection is revealed as the `TemporarySelectionChange` is destroyed.

* Source/WebCore/editing/Editor.cpp:
(WebCore::TemporarySelectionChange::setSelection):

To fix, do not pass in the user-triggered option to `FrameView::defaultSetSelectionOptions`.
Instead, only add the `FrameSelection::IsUserTriggered` option if the temporary
selection change is "user-triggered". This ensures there is no unexpected
application of `RevealSelection` or `FireSelectEvent`.

* Source/WebCore/page/FrameView.cpp:

Continue using `RevealSelection` rather `DelegateMainFrameScroll` for the Scroll
To Text Fragment functionality, as existing tests depend on this behavior. Even
though `DelegateMainFrameScroll` was being specified here, it was being overwritten
by the `defaultSetSelectionOptions` with the user-triggered option. Since STTF
was introduced after 250836 at main, preserve the current behavior. Adopting
`DelegateMainFrameScroll` should be considered in a subsequent patch.

(WebCore::FrameView::scrollToFragment):
(WebCore::FrameView::textFragmentIndicatorTimerFired):
(WebCore::FrameView::scrollToTextFragmentRange):

* Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:
(TEST):

Add an API test to exercise the failing scenario.

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