Skip to content

Commit fc12bec

Browse files
Renzo-OlivaresRenzo Olivares
andauthored
Reland "SliverEnsureSemantics (#165589)" (#166889)
This reverts commit 2fc716d, and updates the cross-axis size of the `_scrollOverflowElement` to be 1px (non-zero), so it is taken into account by the scrollable elements scrollHeight. Fixes #160217 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. --------- Co-authored-by: Renzo Olivares <roliv@google.com>
1 parent 6acc8d3 commit fc12bec

10 files changed

Lines changed: 551 additions & 195 deletions

File tree

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart

Lines changed: 85 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright 2013 The Flutter Authors. All rights reserved.
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
4+
import 'dart:typed_data';
45

56
import 'package:meta/meta.dart';
67
import 'package:ui/src/engine.dart';
@@ -9,20 +10,10 @@ import 'package:ui/ui.dart' as ui;
910
/// Implements vertical and horizontal scrolling functionality for semantics
1011
/// objects.
1112
///
12-
/// Scrolling is implemented using a "joystick" method. The absolute value of
13-
/// "scrollTop" in HTML is not important. We only need to know in whether the
14-
/// value changed in the positive or negative direction. If it changes in the
15-
/// positive direction we send a [ui.SemanticsAction.scrollUp]. Otherwise, we
16-
/// send [ui.SemanticsAction.scrollDown]. The actual scrolling is then handled
17-
/// by the framework and we receive a [ui.SemanticsUpdate] containing the new
18-
/// [scrollPosition] and child positions.
19-
///
20-
/// "scrollTop" or "scrollLeft" is always reset to an arbitrarily chosen non-
21-
/// zero "neutral" scroll position value. This is done so we have a
22-
/// predictable range of DOM scroll position values. When the amount of
23-
/// contents is less than the size of the viewport the browser snaps
24-
/// "scrollTop" back to zero. If there is more content than available in the
25-
/// viewport "scrollTop" may take positive values.
13+
/// Scrolling is controlled by sending the current DOM scroll position in a
14+
/// [ui.SemanticsAction.scrollToOffset] to the framework where it applies the
15+
/// value to its scrollable and the engine receives a [ui.SemanticsUpdate]
16+
/// containing the new [SemanticsObject.scrollPosition] and child positions.
2617
class SemanticScrollable extends SemanticRole {
2718
SemanticScrollable(SemanticsObject semanticsObject)
2819
: super.withBasics(
@@ -39,81 +30,61 @@ class SemanticScrollable extends SemanticRole {
3930
/// Disables browser-driven scrolling in the presence of pointer events.
4031
GestureModeCallback? _gestureModeListener;
4132

42-
/// DOM element used as a workaround for: https://github.com/flutter/flutter/issues/104036
43-
///
44-
/// When the assistive technology gets to the last element of the scrollable
45-
/// list, the browser thinks the scrollable area doesn't have any more content,
46-
/// so it overrides the value of "scrollTop"/"scrollLeft" with zero. As a result,
47-
/// the user can't scroll back up/left.
48-
///
49-
/// As a workaround, we add this DOM element and set its size to
50-
/// [canonicalNeutralScrollPosition] so the browser believes
51-
/// that the scrollable area still has some more content, and doesn't override
52-
/// scrollTop/scrollLetf with zero.
33+
/// DOM element used to indicate to the browser the total quantity of available
34+
/// content under this scrollable area. This element is sized based on the
35+
/// total scroll extent calculated by scrollExtentMax - scrollExtentMin + rect.height
36+
/// of the [SemanticsObject] managed by this scrollable.
5337
final DomElement _scrollOverflowElement = createDomElement('flt-semantics-scroll-overflow');
5438

5539
/// Listens to HTML "scroll" gestures detected by the browser.
5640
///
57-
/// This gesture is converted to [ui.SemanticsAction.scrollUp] or
58-
/// [ui.SemanticsAction.scrollDown], depending on the direction.
41+
/// When the browser detects a "scroll" gesture we send the updated DOM scroll position
42+
/// to the framework in a [ui.SemanticsAction.scrollToOffset].
5943
@visibleForTesting
6044
DomEventListener? scrollListener;
6145

62-
/// The value of the "scrollTop" or "scrollLeft" property of this object's
63-
/// [element] that has zero offset relative to the [scrollPosition].
64-
int _effectiveNeutralScrollPosition = 0;
65-
6646
/// Whether this scrollable can scroll vertically or horizontally.
6747
bool get _canScroll =>
6848
semanticsObject.isVerticalScrollContainer || semanticsObject.isHorizontalScrollContainer;
6949

50+
/// The previous value of the "scrollTop" or "scrollLeft" property of this object's
51+
/// [element], used to determine if the content was scrolled.
52+
int _previousDomScrollPosition = 0;
53+
7054
/// Responds to browser-detected "scroll" gestures.
7155
void _recomputeScrollPosition() {
72-
if (_domScrollPosition != _effectiveNeutralScrollPosition) {
56+
if (_domScrollPosition != _previousDomScrollPosition) {
7357
if (!EngineSemantics.instance.shouldAcceptBrowserGesture('scroll')) {
7458
return;
7559
}
76-
final bool doScrollForward = _domScrollPosition > _effectiveNeutralScrollPosition;
77-
_neutralizeDomScrollPosition();
60+
61+
_previousDomScrollPosition = _domScrollPosition;
62+
_updateScrollableState();
7863
semanticsObject.recomputePositionAndSize();
7964
semanticsObject.updateChildrenPositionAndSize();
8065

8166
final int semanticsId = semanticsObject.id;
82-
if (doScrollForward) {
83-
if (semanticsObject.isVerticalScrollContainer) {
84-
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
85-
viewId,
86-
semanticsId,
87-
ui.SemanticsAction.scrollUp,
88-
null,
89-
);
90-
} else {
91-
assert(semanticsObject.isHorizontalScrollContainer);
92-
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
93-
viewId,
94-
semanticsId,
95-
ui.SemanticsAction.scrollLeft,
96-
null,
97-
);
98-
}
67+
final Float64List offsets = Float64List(2);
68+
69+
// Either SemanticsObject.isVerticalScrollContainer or
70+
// SemanticsObject.isHorizontalScrollContainer should be
71+
// true otherwise scrollToOffset cannot be called.
72+
if (semanticsObject.isVerticalScrollContainer) {
73+
offsets[0] = 0.0;
74+
offsets[1] = element.scrollTop;
9975
} else {
100-
if (semanticsObject.isVerticalScrollContainer) {
101-
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
102-
viewId,
103-
semanticsId,
104-
ui.SemanticsAction.scrollDown,
105-
null,
106-
);
107-
} else {
108-
assert(semanticsObject.isHorizontalScrollContainer);
109-
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
110-
viewId,
111-
semanticsId,
112-
ui.SemanticsAction.scrollRight,
113-
null,
114-
);
115-
}
76+
assert(semanticsObject.isHorizontalScrollContainer);
77+
offsets[0] = element.scrollLeft;
78+
offsets[1] = 0.0;
11679
}
80+
81+
final ByteData? message = const StandardMessageCodec().encodeMessage(offsets);
82+
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
83+
viewId,
84+
semanticsId,
85+
ui.SemanticsAction.scrollToOffset,
86+
message,
87+
);
11788
}
11889
}
11990

@@ -122,6 +93,22 @@ class SemanticScrollable extends SemanticRole {
12293
// Scrolling is controlled by setting overflow-y/overflow-x to 'scroll`. The
12394
// default overflow = "visible" needs to be unset.
12495
semanticsObject.element.style.overflow = '';
96+
// On macOS the scrollbar behavior which can be set in the settings application
97+
// may sometimes insert scrollbars into an application when a peripheral like a
98+
// mouse or keyboard is plugged in. This causes the clientHeight or clientWidth
99+
// of the scrollable DOM element to be offset by the width of the scrollbar.
100+
// This causes issues in the vertical scrolling context because the max scroll
101+
// extent is calculated by the element's scrollHeight - clientHeight, so when
102+
// the clientHeight is offset by scrollbar width the browser may there is
103+
// a greater scroll extent then what is actually available.
104+
//
105+
// The scrollbar is already made transparent in SemanticsRole._initElement so here
106+
// set scrollbar-width to "none" to prevent it from affecting the max scroll extent.
107+
//
108+
// Support for scrollbar-width was only added to Safari v18.2+, so versions before
109+
// that may still experience overscroll issues when macOS inserts scrollbars
110+
// into the application.
111+
semanticsObject.element.style.scrollbarWidth = 'none';
125112

126113
_scrollOverflowElement.style
127114
..position = 'absolute'
@@ -136,7 +123,15 @@ class SemanticScrollable extends SemanticRole {
136123
super.update();
137124

138125
semanticsObject.owner.addOneTimePostUpdateCallback(() {
139-
_neutralizeDomScrollPosition();
126+
if (_canScroll) {
127+
final double? scrollPosition = semanticsObject.scrollPosition;
128+
assert(scrollPosition != null);
129+
if (scrollPosition != _domScrollPosition) {
130+
element.scrollTop = scrollPosition!;
131+
_previousDomScrollPosition = _domScrollPosition;
132+
}
133+
}
134+
_updateScrollableState();
140135
semanticsObject.recomputePositionAndSize();
141136
semanticsObject.updateChildrenPositionAndSize();
142137
});
@@ -183,64 +178,49 @@ class SemanticScrollable extends SemanticRole {
183178
}
184179
}
185180

186-
/// Resets the scroll position (top or left) to the neutral value.
187-
///
188-
/// The scroll position of the scrollable HTML node that's considered to
189-
/// have zero offset relative to Flutter's notion of scroll position is
190-
/// referred to as "neutral scroll position".
191-
///
192-
/// We always set the scroll position to a non-zero value in order to
193-
/// be able to scroll in the negative direction. When scrollTop/scrollLeft is
194-
/// zero the browser will refuse to scroll back even when there is more
195-
/// content available.
196-
void _neutralizeDomScrollPosition() {
181+
void _updateScrollableState() {
197182
// This value is arbitrary.
198-
const int canonicalNeutralScrollPosition = 10;
199183
final ui.Rect? rect = semanticsObject.rect;
200184
if (rect == null) {
201185
printWarning('Warning! the rect attribute of semanticsObject is null');
202186
return;
203187
}
188+
final double? scrollExtentMax = semanticsObject.scrollExtentMax;
189+
final double? scrollExtentMin = semanticsObject.scrollExtentMin;
190+
assert(scrollExtentMax != null);
191+
assert(scrollExtentMin != null);
192+
final double scrollExtentTotal =
193+
scrollExtentMax! -
194+
scrollExtentMin! +
195+
(semanticsObject.isVerticalScrollContainer ? rect.height : rect.width);
196+
// Place the _scrollOverflowElement at the beginning of the content
197+
// and size it based on the total scroll extent so the browser
198+
// knows how much scrollable content there is.
204199
if (semanticsObject.isVerticalScrollContainer) {
205-
// Place the _scrollOverflowElement at the end of the content and
206-
// make sure that when we neutralize the scrolling position,
207-
// it doesn't scroll into the visible area.
208-
final int verticalOffset = rect.height.ceil() + canonicalNeutralScrollPosition;
209200
_scrollOverflowElement.style
210-
..transform = 'translate(0px,${verticalOffset}px)'
211-
..width = '${rect.width.round()}px'
212-
..height = '${canonicalNeutralScrollPosition}px';
213-
214-
element.scrollTop = canonicalNeutralScrollPosition.toDouble();
215-
// Read back because the effective value depends on the amount of content.
216-
_effectiveNeutralScrollPosition = element.scrollTop.toInt();
201+
// The cross axis size should be non-zero so it is taken into
202+
// account in the scrollable elements scrollHeight.
203+
..width = '1px'
204+
..height = '${scrollExtentTotal.toStringAsFixed(1)}px';
217205
semanticsObject
218-
..verticalScrollAdjustment = _effectiveNeutralScrollPosition.toDouble()
206+
..verticalScrollAdjustment = element.scrollTop
219207
..horizontalScrollAdjustment = 0.0;
220208
} else if (semanticsObject.isHorizontalScrollContainer) {
221-
// Place the _scrollOverflowElement at the end of the content and
222-
// make sure that when we neutralize the scrolling position,
223-
// it doesn't scroll into the visible area.
224-
final int horizontalOffset = rect.width.ceil() + canonicalNeutralScrollPosition;
225209
_scrollOverflowElement.style
226-
..transform = 'translate(${horizontalOffset}px,0px)'
227-
..width = '${canonicalNeutralScrollPosition}px'
228-
..height = '${rect.height.round()}px';
229-
230-
element.scrollLeft = canonicalNeutralScrollPosition.toDouble();
231-
// Read back because the effective value depends on the amount of content.
232-
_effectiveNeutralScrollPosition = element.scrollLeft.toInt();
210+
..width = '${scrollExtentTotal.toStringAsFixed(1)}px'
211+
// The cross axis size should be non-zero so it is taken into
212+
// account in the scrollable elements scrollHeight.
213+
..height = '1px';
233214
semanticsObject
234215
..verticalScrollAdjustment = 0.0
235-
..horizontalScrollAdjustment = _effectiveNeutralScrollPosition.toDouble();
216+
..horizontalScrollAdjustment = element.scrollLeft;
236217
} else {
237218
_scrollOverflowElement.style
238219
..transform = 'translate(0px,0px)'
239220
..width = '0px'
240221
..height = '0px';
241222
element.scrollLeft = 0.0;
242223
element.scrollTop = 0.0;
243-
_effectiveNeutralScrollPosition = 0;
244224
semanticsObject
245225
..verticalScrollAdjustment = 0.0
246226
..horizontalScrollAdjustment = 0.0;

0 commit comments

Comments
 (0)