Skip to content
Permalink
Browse files
Scroll To Text Fragments fragments should alwasy be scrolled to the c…
…ente of the screen.

https://bugs.webkit.org/show_bug.cgi?id=245169
<rdar://99904303>

Reviewed by Tim Horton.

Our code that scrolls selections into visible does not scroll them if they are already visible
at the current page scroll. We are wanting this to always go to the center so that it is easier
to hold the position, rather than have the scrolled text jump around a bit as additional content
is loaded in.

* Source/WebCore/editing/Editor.cpp:
(WebCore::TemporarySelectionChange::setSelection):
* Source/WebCore/editing/Editor.h:
* Source/WebCore/editing/FrameSelection.cpp:
(WebCore::FrameSelection::setSelection):
(WebCore::FrameSelection::updateAndRevealSelection):
* Source/WebCore/editing/FrameSelection.h:
* Source/WebCore/page/FrameView.cpp:
(WebCore::FrameView::scrollToFragment):
(WebCore::FrameView::textFragmentIndicatorTimerFired):

Canonical link: https://commits.webkit.org/254477@main
  • Loading branch information
megangardner committed Sep 14, 2022
1 parent d6faf5b commit 6ae0bcb1c73ce6ea808e09557db62a0dbc5446d6
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 7 deletions.
@@ -278,6 +278,8 @@ void TemporarySelectionChange::setSelection(const VisibleSelection& selection, I
options.add(FrameSelection::SmoothScroll);
if (m_options & TemporarySelectionOption::RevealSelectionBounds)
options.add(FrameSelection::RevealSelectionBounds);
if (m_options & TemporarySelectionOption::ForceCenterScroll)
options.add(FrameSelection::ForceCenterScroll);
}

m_document->selection().setSelection(selection, options);
@@ -111,7 +111,7 @@ enum class MailBlockquoteHandling {
class HTMLAttachmentElement;
#endif

enum class TemporarySelectionOption : uint8_t {
enum class TemporarySelectionOption : uint16_t {
RevealSelection = 1 << 0,
DoNotSetFocus = 1 << 1,

@@ -128,6 +128,8 @@ enum class TemporarySelectionOption : uint8_t {
RevealSelectionBounds = 1 << 6,

UserTriggered = 1 << 7,

ForceCenterScroll = 1 << 8,
};

class TemporarySelectionChange {
@@ -466,7 +466,7 @@ void FrameSelection::setSelection(const VisibleSelection& selection, OptionSet<S
return;
}

updateAndRevealSelection(intent, options.contains(SmoothScroll) ? ScrollBehavior::Smooth : ScrollBehavior::Instant, options.contains(RevealSelectionBounds) ? RevealExtentOption::DoNotRevealExtent : RevealExtentOption::RevealExtent);
updateAndRevealSelection(intent, options.contains(SmoothScroll) ? ScrollBehavior::Smooth : ScrollBehavior::Instant, options.contains(RevealSelectionBounds) ? RevealExtentOption::DoNotRevealExtent : RevealExtentOption::RevealExtent, options.contains(ForceCenterScroll) ? ForceCenterScrollOption::ForceCenterScroll : ForceCenterScrollOption::DoNotForceCenterScroll);

if (options & IsUserTriggered) {
if (auto* client = protectedDocument->editor().client())
@@ -499,7 +499,7 @@ void FrameSelection::setNeedsSelectionUpdate(RevealSelectionAfterUpdate revealMo
view->selection().clear();
}

void FrameSelection::updateAndRevealSelection(const AXTextStateChangeIntent& intent, ScrollBehavior scrollBehavior, RevealExtentOption revealExtent)
void FrameSelection::updateAndRevealSelection(const AXTextStateChangeIntent& intent, ScrollBehavior scrollBehavior, RevealExtentOption revealExtent, ForceCenterScrollOption forceCenterScroll)
{
if (!m_pendingSelectionUpdate)
return;
@@ -515,6 +515,9 @@ void FrameSelection::updateAndRevealSelection(const AXTextStateChangeIntent& int
alignment = m_alwaysAlignCursorOnScrollWhenRevealingSelection ? ScrollAlignment::alignCenterAlways : ScrollAlignment::alignCenterIfNeeded;
else
alignment = m_alwaysAlignCursorOnScrollWhenRevealingSelection ? ScrollAlignment::alignTopAlways : ScrollAlignment::alignToEdgeIfNeeded;

if (forceCenterScroll == ForceCenterScrollOption::ForceCenterScroll)
alignment = ScrollAlignment::alignCenterAlways;

revealSelection(m_selectionRevealMode, alignment, revealExtent, scrollBehavior);
}
@@ -52,6 +52,7 @@ class VisiblePosition;

enum EUserTriggered : bool { NotUserTriggered, UserTriggered };
enum RevealExtentOption : bool { RevealExtent, DoNotRevealExtent };
enum ForceCenterScrollOption : bool { DoNotForceCenterScroll, ForceCenterScroll };

class CaretBase {
WTF_MAKE_NONCOPYABLE(CaretBase);
@@ -128,7 +129,8 @@ class FrameSelection : private CaretBase {
RevealSelectionUpToMainFrame = 1 << 8,
SmoothScroll = 1 << 9,
DelegateMainFrameScroll = 1 << 10,
RevealSelectionBounds = 1 << 11
RevealSelectionBounds = 1 << 11,
ForceCenterScroll = 1 << 12,
};
static constexpr OptionSet<SetSelectionOption> defaultSetSelectionOptions(EUserTriggered = NotUserTriggered);

@@ -268,7 +270,7 @@ class FrameSelection : private CaretBase {

private:
void updateSelectionAppearanceNow();
void updateAndRevealSelection(const AXTextStateChangeIntent&, ScrollBehavior = ScrollBehavior::Instant, RevealExtentOption = RevealExtentOption::RevealExtent);
void updateAndRevealSelection(const AXTextStateChangeIntent&, ScrollBehavior = ScrollBehavior::Instant, RevealExtentOption = RevealExtentOption::RevealExtent, ForceCenterScrollOption = ForceCenterScrollOption::DoNotForceCenterScroll);
void updateDataDetectorsForSelection();

bool setSelectionWithoutUpdatingAppearance(const VisibleSelection&, OptionSet<SetSelectionOption>, CursorAlignOnScroll, TextGranularity);
@@ -2228,7 +2228,7 @@ bool FrameView::scrollToFragment(const URL& url)

if (highlightRanges.size()) {
auto range = highlightRanges.first();
TemporarySelectionChange selectionChange(document, { range }, { TemporarySelectionOption::DelegateMainFrameScroll, TemporarySelectionOption::RevealSelectionBounds, TemporarySelectionOption::UserTriggered });
TemporarySelectionChange selectionChange(document, { range }, { TemporarySelectionOption::DelegateMainFrameScroll, TemporarySelectionOption::RevealSelectionBounds, TemporarySelectionOption::UserTriggered, TemporarySelectionOption::ForceCenterScroll });
m_pendingTextFragmentIndicatorRange = range;
m_pendingTextFragmentIndicatorText = plainText(range);
if (frame().settings().scrollToTextFragmentIndicatorEnabled())
@@ -2464,7 +2464,7 @@ void FrameView::textFragmentIndicatorTimerFired()
return;

auto range = m_pendingTextFragmentIndicatorRange.value();
TemporarySelectionChange selectionChange(document, { range }, { TemporarySelectionOption::DelegateMainFrameScroll, TemporarySelectionOption::RevealSelectionBounds, TemporarySelectionOption::UserTriggered });
TemporarySelectionChange selectionChange(document, { range }, { TemporarySelectionOption::DelegateMainFrameScroll, TemporarySelectionOption::RevealSelectionBounds, TemporarySelectionOption::UserTriggered, TemporarySelectionOption::ForceCenterScroll });

auto textIndicator = TextIndicator::createWithRange(range, { TextIndicatorOption::DoNotClipToVisibleRect }, WebCore::TextIndicatorPresentationTransition::Bounce);
if (textIndicator)

0 comments on commit 6ae0bcb

Please sign in to comment.