Skip to content

Commit 2fc716d

Browse files
auto-submit[bot]auto-submit[bot]
and
auto-submit[bot]
authored
Reverts "SliverEnsureSemantics (flutter#165589)" (flutter#166870)
<!-- start_original_pr_link --> Reverts: flutter#165589 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: Renzo-Olivares <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: breaking internal tests <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: Renzo-Olivares <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {Piinks} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: Currently when using a `CustomScrollView`, screen readers cannot list or move focus to elements that are outside the current Viewport and cache extent because we do not create semantic nodes for these elements. This change introduces `SliverEnsureSemantics` which ensures its sliver child is included in the semantics tree, whether or not it is currently visible on the screen or within the cache extent. This way screen readers are aware the elements are there and can navigate to them / create accessibility traversal menus with this information. * Under the hood a new flag has been added to `RenderSliver` called `ensureSemantics`. `RenderViewportBase` uses this in its `visitChildrenForSemantics` to ensure a sliver is visited when creating the semantics tree. Previously a sliver was not visited if it was not visible or within the cache extent. `RenderViewportBase` also uses this in `describeSemanticsClip` and `describeApproximatePaintClip` to ensure a sliver child that wants to "ensure semantics" is not clipped out if it is not currently visible in the viewport or outside the cache extent. * `RenderSliverMultiBoxAdaptor.semanticBounds` now leverages its first child as an anchor for assistive technologies to be able to reach it if the Sliver is a child of `SliverEnsureSemantics`. If not it will still be dropped from the semantics tree. * `RenderProxySliver` now considers child overrides of `semanticBounds`. On the engine side we move from using a joystick method to scroll with `SemanticsAction.scrollUp` and `SemanticsAction.scrollDown` to using `SemanticsAction.scrollToOffset` completely letting the browser drive the scrolling with its current dom scroll position "scrollTop" or "scrollLeft". This is possible by calculating the total quantity of content under the scrollable and sizing the scroll element based on that. <details open><summary>Code sample</summary> ```dart // Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; /// Flutter code sample for [SliverEnsureSemantics]. void main() => runApp(const SliverEnsureSemanticsExampleApp()); class SliverEnsureSemanticsExampleApp extends StatelessWidget { const SliverEnsureSemanticsExampleApp({super.key}); @OverRide Widget build(BuildContext context) { return const MaterialApp(home: SliverEnsureSemanticsExample()); } } class SliverEnsureSemanticsExample extends StatefulWidget { const SliverEnsureSemanticsExample({super.key}); @OverRide State<SliverEnsureSemanticsExample> createState() => _SliverEnsureSemanticsExampleState(); } class _SliverEnsureSemanticsExampleState extends State<SliverEnsureSemanticsExample> { @OverRide Widget build(BuildContext context) { final ThemeData theme = Theme.of(context); return Scaffold( appBar: AppBar( backgroundColor: theme.colorScheme.inversePrimary, title: const Text('SliverEnsureSemantics Demo'), ), body: Center( child: CustomScrollView( semanticChildCount: 106, slivers: <Widget>[ SliverEnsureSemantics( sliver: SliverToBoxAdapter( child: IndexedSemantics( index: 0, child: Card( child: Padding( padding: const EdgeInsets.all(8.0), child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: <Widget>[ Semantics( header: true, headingLevel: 3, child: Text( 'Steps to reproduce', style: theme.textTheme.headlineSmall, ), ), const Text('Issue description'), Semantics( header: true, headingLevel: 3, child: Text( 'Expected Results', style: theme.textTheme.headlineSmall, ), ), Semantics( header: true, headingLevel: 3, child: Text( 'Actual Results', style: theme.textTheme.headlineSmall, ), ), Semantics( header: true, headingLevel: 3, child: Text( 'Code Sample', style: theme.textTheme.headlineSmall, ), ), Semantics( header: true, headingLevel: 3, child: Text( 'Screenshots', style: theme.textTheme.headlineSmall, ), ), Semantics( header: true, headingLevel: 3, child: Text( 'Logs', style: theme.textTheme.headlineSmall, ), ), ], ), ), ), ), ), ), SliverFixedExtentList( itemExtent: 44.0, delegate: SliverChildBuilderDelegate( (BuildContext context, int index) { return Card( child: Padding( padding: const EdgeInsets.all(8.0), child: Text('Item $index'), ), ); }, childCount: 50, semanticIndexOffset: 1, ), ), SliverEnsureSemantics( sliver: SliverToBoxAdapter( child: IndexedSemantics( index: 51, child: Card( child: Padding( padding: const EdgeInsets.all(8.0), child: Semantics( header: true, child: const Text('Footer 1'), ), ), ), ), ), ), SliverEnsureSemantics( sliver: SliverToBoxAdapter( child: IndexedSemantics( index: 52, child: Card( child: Padding( padding: const EdgeInsets.all(8.0), child: Semantics( header: true, child: const Text('Footer 2'), ), ), ), ), ), ), SliverEnsureSemantics( sliver: SliverToBoxAdapter( child: IndexedSemantics( index: 53, child: Semantics(link: true, child: const Text('Link #1')), ), ), ), SliverEnsureSemantics( sliver: SliverToBoxAdapter( child: IndexedSemantics( index: 54, child: OverflowBar( children: <Widget>[ TextButton( onPressed: () {}, child: const Text('Button 1'), ), TextButton( onPressed: () {}, child: const Text('Button 2'), ), ], ), ), ), ), SliverEnsureSemantics( sliver: SliverToBoxAdapter( child: IndexedSemantics( index: 55, child: Semantics(link: true, child: const Text('Link #2')), ), ), ), SliverEnsureSemantics( sliver: SliverSemanticsList( sliver: SliverFixedExtentList( itemExtent: 44.0, delegate: SliverChildBuilderDelegate( (BuildContext context, int index) { return Semantics( role: SemanticsRole.listItem, child: Card( child: Padding( padding: const EdgeInsets.all(8.0), child: Text('Second List Item $index'), ), ), ); }, childCount: 50, semanticIndexOffset: 56, ), ), ), ), SliverEnsureSemantics( sliver: SliverToBoxAdapter( child: IndexedSemantics( index: 107, child: Semantics(link: true, child: const Text('Link #3')), ), ), ), ], ), ), ); } } // A sliver that assigns the role of SemanticsRole.list to its sliver child. class SliverSemanticsList extends SingleChildRenderObjectWidget { const SliverSemanticsList({super.key, required Widget sliver}) : super(child: sliver); @OverRide RenderSliverSemanticsList createRenderObject(BuildContext context) => RenderSliverSemanticsList(); } class RenderSliverSemanticsList extends RenderProxySliver { @OverRide void describeSemanticsConfiguration(SemanticsConfiguration config) { super.describeSemanticsConfiguration(config); config.role = SemanticsRole.list; } } ``` </details> Fixes: flutter#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. <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
1 parent 059326d commit 2fc716d

File tree

10 files changed

+195
-547
lines changed

10 files changed

+195
-547
lines changed

Diff for: engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart

+105-81
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
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';
54

65
import 'package:meta/meta.dart';
76
import 'package:ui/src/engine.dart';
@@ -10,10 +9,20 @@ import 'package:ui/ui.dart' as ui;
109
/// Implements vertical and horizontal scrolling functionality for semantics
1110
/// objects.
1211
///
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.
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.
1726
class SemanticScrollable extends SemanticRole {
1827
SemanticScrollable(SemanticsObject semanticsObject)
1928
: super.withBasics(
@@ -30,61 +39,81 @@ class SemanticScrollable extends SemanticRole {
3039
/// Disables browser-driven scrolling in the presence of pointer events.
3140
GestureModeCallback? _gestureModeListener;
3241

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.
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.
3753
final DomElement _scrollOverflowElement = createDomElement('flt-semantics-scroll-overflow');
3854

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

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+
4666
/// Whether this scrollable can scroll vertically or horizontally.
4767
bool get _canScroll =>
4868
semanticsObject.isVerticalScrollContainer || semanticsObject.isHorizontalScrollContainer;
4969

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-
5470
/// Responds to browser-detected "scroll" gestures.
5571
void _recomputeScrollPosition() {
56-
if (_domScrollPosition != _previousDomScrollPosition) {
72+
if (_domScrollPosition != _effectiveNeutralScrollPosition) {
5773
if (!EngineSemantics.instance.shouldAcceptBrowserGesture('scroll')) {
5874
return;
5975
}
60-
61-
_previousDomScrollPosition = _domScrollPosition;
62-
_updateScrollableState();
76+
final bool doScrollForward = _domScrollPosition > _effectiveNeutralScrollPosition;
77+
_neutralizeDomScrollPosition();
6378
semanticsObject.recomputePositionAndSize();
6479
semanticsObject.updateChildrenPositionAndSize();
6580

6681
final int semanticsId = semanticsObject.id;
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;
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+
}
7599
} else {
76-
assert(semanticsObject.isHorizontalScrollContainer);
77-
offsets[0] = element.scrollLeft;
78-
offsets[1] = 0.0;
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+
}
79116
}
80-
81-
final ByteData? message = const StandardMessageCodec().encodeMessage(offsets);
82-
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
83-
viewId,
84-
semanticsId,
85-
ui.SemanticsAction.scrollToOffset,
86-
message,
87-
);
88117
}
89118
}
90119

@@ -93,22 +122,6 @@ class SemanticScrollable extends SemanticRole {
93122
// Scrolling is controlled by setting overflow-y/overflow-x to 'scroll`. The
94123
// default overflow = "visible" needs to be unset.
95124
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';
112125

113126
_scrollOverflowElement.style
114127
..position = 'absolute'
@@ -123,15 +136,7 @@ class SemanticScrollable extends SemanticRole {
123136
super.update();
124137

125138
semanticsObject.owner.addOneTimePostUpdateCallback(() {
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();
139+
_neutralizeDomScrollPosition();
135140
semanticsObject.recomputePositionAndSize();
136141
semanticsObject.updateChildrenPositionAndSize();
137142
});
@@ -178,45 +183,64 @@ class SemanticScrollable extends SemanticRole {
178183
}
179184
}
180185

181-
void _updateScrollableState() {
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() {
182197
// This value is arbitrary.
198+
const int canonicalNeutralScrollPosition = 10;
183199
final ui.Rect? rect = semanticsObject.rect;
184200
if (rect == null) {
185201
printWarning('Warning! the rect attribute of semanticsObject is null');
186202
return;
187203
}
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.
199204
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;
200209
_scrollOverflowElement.style
201-
..width = '0px'
202-
..height = '${scrollExtentTotal.toStringAsFixed(1)}px';
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();
203217
semanticsObject
204-
..verticalScrollAdjustment = element.scrollTop
218+
..verticalScrollAdjustment = _effectiveNeutralScrollPosition.toDouble()
205219
..horizontalScrollAdjustment = 0.0;
206220
} 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;
207225
_scrollOverflowElement.style
208-
..width = '${scrollExtentTotal.toStringAsFixed(1)}px'
209-
..height = '0px';
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();
210233
semanticsObject
211234
..verticalScrollAdjustment = 0.0
212-
..horizontalScrollAdjustment = element.scrollLeft;
235+
..horizontalScrollAdjustment = _effectiveNeutralScrollPosition.toDouble();
213236
} else {
214237
_scrollOverflowElement.style
215238
..transform = 'translate(0px,0px)'
216239
..width = '0px'
217240
..height = '0px';
218241
element.scrollLeft = 0.0;
219242
element.scrollTop = 0.0;
243+
_effectiveNeutralScrollPosition = 0;
220244
semanticsObject
221245
..verticalScrollAdjustment = 0.0
222246
..horizontalScrollAdjustment = 0.0;

0 commit comments

Comments
 (0)