Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make SemanticsNode.isMergedIntoParent Readonly #137304

Merged
merged 5 commits into from Oct 27, 2023
Merged

Make SemanticsNode.isMergedIntoParent Readonly #137304

merged 5 commits into from Oct 27, 2023

Conversation

LongCatIsLooong
Copy link
Contributor

Fixes #54665

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: scrolling Viewports, list views, slivers, etc. labels Oct 26, 2023
@LongCatIsLooong LongCatIsLooong marked this pull request as draft October 26, 2023 01:23
@flutter-dashboard

This comment was marked as outdated.

Sign in to view
@LongCatIsLooong LongCatIsLooong marked this pull request as ready for review October 26, 2023 04:52
@LongCatIsLooong LongCatIsLooong marked this pull request as draft October 26, 2023 20:30
@LongCatIsLooong LongCatIsLooong marked this pull request as ready for review October 26, 2023 22:50
goderbauer
goderbauer reviewed Oct 26, 2023
View reviewed changes
packages/flutter/lib/src/rendering/paragraph.dart
@@ -1230,7 +1230,7 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
final Rect paintRect = node.parentPaintClipRect!.intersect(currentRect);
configuration.isHidden = paintRect.isEmpty && !currentRect.isEmpty;
}
late final SemanticsNode newChild;
final SemanticsNode newChild;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I wish we had a lint for this, dart-lang/linter#4365)

packages/flutter/lib/src/semantics/semantics.dart Outdated
@@ -2045,6 +2041,28 @@ class SemanticsNode with DiagnosticableTreeMixin {
_children?.forEach(_redepthChild);
}

void _updateChildMergeFlag(SemanticsNode child) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a recursively to this method name to indicate that it will also do so for the child's children (similarly to other methods like that that we have in the framework).

packages/flutter/lib/src/semantics/semantics.dart Outdated
Comment on lines 3317 to 3321
if (!rootSemanticsNode.mergeAllDescendantsIntoThisNode) {
rootSemanticsNode.visitChildren(findInvisibleNodes);
} else if (rootSemanticsNode.childrenCount > 0 && rootSemanticsNode.rect.isEmpty) {
invisibleNodes.add(rootSemanticsNode);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the rootSemanticsNode also be considered invisible if it doesn't merge all descendants into it, has a childrenCount > 0, and an empty rect?

In other words, in the if branch, why is the rootNode not considered invisible if it has children and and empty rect?

packages/flutter/lib/src/semantics/semantics.dart Outdated
// Once the tree is up-to-date, verify that there's no invisible nodes in it.
assert(() {
final List<SemanticsNode> invisibleNodes = <SemanticsNode>[];
bool findInvisibleNodes(SemanticsNode rootNode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename the name rootNode to just node here? It gets a little confusing because this is not necessarily gonna be the root root node for every call?

goderbauer
goderbauer approved these changes Oct 27, 2023
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 27, 2023
@auto-submit auto-submit bot merged commit 7a3135b into flutter:master Oct 27, 2023
61 checks passed
@LongCatIsLooong LongCatIsLooong deleted the make-isMergedIntoParent-readonly branch October 27, 2023 17:45
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 27, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 27, 2023
@engine-flutter-autoroll
7a3135b9c Make SemanticsNode.isMergedIntoParent Readonly (flutter/f…
c48bcf1 
…lutter#137304)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 27, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 27, 2023
@engine-flutter-autoroll
7a3135b9c Make SemanticsNode.isMergedIntoParent Readonly (flutter/f…
a1cc562 
…lutter#137304)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Oct 27, 2023
@engine-flutter-autoroll
Roll Flutter from c555599fbb4a to 5907c9757e3f (45 revisions) (#5252)
8c1c865 
Roll Flutter from c555599fbb4a to 5907c9757e3f (45 revisions)

flutter/flutter@c555599...5907c97

2023-10-27 gspencergoog@users.noreply.github.com Add `isLogicalKeyPressed` to `KeyEvent` (flutter/flutter#136856)
2023-10-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Ensure `flutter build apk --release` optimizes+shrinks platform code" (flutter/flutter#137433)
2023-10-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from bedc49efc85c to a198ad4e740d (20 revisions) (flutter/flutter#137429)
2023-10-27 31859944+LongCatIsLooong@users.noreply.github.com Make `SemanticsNode.isMergedIntoParent` Readonly (flutter/flutter#137304)
2023-10-27 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from bedc49efc85c to 71e1a0430232 (16 revisions) (flutter/flutter#137422)
2023-10-27 engine-flutter-autoroll@skia.org Roll Packages from fea24c5 to 2af6954 (5 revisions) (flutter/flutter#137421)
2023-10-27 15619084+vashworth@users.noreply.github.com Run tests on either macOS 12 or 13 (flutter/flutter#137365)
2023-10-27 tessertaha@gmail.com Revert "Update `DataTable` test when data row is pressed for Material 3 (#137230)" (flutter/flutter#137407)
2023-10-27 tessertaha@gmail.com Revert "Reland - Update `OutlinedButton` tests for Material 3 (#136809) (#137247)" (flutter/flutter#137406)
2023-10-27 kustermann@google.com Ensure `flutter build apk --release` optimizes+shrinks platform code (flutter/flutter#136880)
2023-10-27 tessertaha@gmail.com Update `DataTable` test when data row is pressed for Material 3 (flutter/flutter#137230)
2023-10-27 tessertaha@gmail.com Reland - Update `OutlinedButton` tests for Material 3 (#136809) (flutter/flutter#137247)
2023-10-27 andrewrkolos@gmail.com update asset manifest file name referenced in `WebServiceWorker` (flutter/flutter#135954)
2023-10-27 andrewrkolos@gmail.com give `throwsToolExit` a more useful description (flutter/flutter#136694)
2023-10-27 tvolkert@users.noreply.github.com Add ConstrainedLayoutBuilder.updateShouldRebuild() (flutter/flutter#136691)
2023-10-27 41873024+droidbg@users.noreply.github.com Fix. typos (flutter/flutter#137325)
2023-10-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 87f384c2d70b to bedc49efc85c (1 revision) (flutter/flutter#137382)
2023-10-26 pateltirth454@gmail.com Fix Typos (flutter/flutter#137292)
2023-10-26 sokolovskyi.konstantin@gmail.com AnimationController should dispatch creation in constructor. (flutter/flutter#134839)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9788bb9ff83e to 87f384c2d70b (4 revisions) (flutter/flutter#137380)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from ce1c1ee54107 to 9788bb9ff83e (3 revisions) (flutter/flutter#137373)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 windows_startup_test to be unflaky (flutter/flutter#137228)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 flutter_tool_startup__windows to be unflaky (flutter/flutter#137229)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 windows_home_scroll_perf__timeline_summary to be unflaky (flutter/flutter#137221)
2023-10-26 34871572+gmackall@users.noreply.github.com Declare dependency on copyFlutterAssetsTask in bundleAarTask (flutter/flutter#137370)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 complex_layout_win_desktop__start_up to be unflaky (flutter/flutter#137225)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 394744d2c4d0 to ce1c1ee54107 (2 revisions) (flutter/flutter#137367)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 hot_mode_dev_cycle_win_target__benchmark to be unflaky (flutter/flutter#137217)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 flutter_view_win_desktop__start_up to be unflaky (flutter/flutter#137226)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 platform_view_win_desktop__start_up to be unflaky (flutter/flutter#137227)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 flutter_gallery_win_desktop__start_up to be unflaky (flutter/flutter#137224)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 hello_world_win_desktop__compile to be unflaky (flutter/flutter#137222)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 flutter_gallery_win_desktop__compile to be unflaky (flutter/flutter#137223)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 run_release_test_windows to be unflaky (flutter/flutter#137220)
2023-10-26 greg@zulip.com Fix dislocated doc and comment on ThemeData localize cache (flutter/flutter#137315)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 platform_channel_sample_test_windows to be unflaky (flutter/flutter#137218)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7c5c8f587992 to 394744d2c4d0 (1 revision) (flutter/flutter#137364)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9363fe6ba503 to 7c5c8f587992 (3 revisions) (flutter/flutter#137363)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 542f8bc4c019 to 9363fe6ba503 (2 revisions) (flutter/flutter#137354)
2023-10-26 42216813+eliasyishak@users.noreply.github.com Unified analytics events for doctor validators (flutter/flutter#136647)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from d8132d5070bf to 542f8bc4c019 (2 revisions) (flutter/flutter#137349)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 run_debug_test_windows to be unflaky (flutter/flutter#137219)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0a6253dbfafd to d8132d5070bf (1 revision) (flutter/flutter#137347)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5da115661f01 to 0a6253dbfafd (1 revision) (flutter/flutter#137341)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 61ae5ef94e9c to 5da115661f01 (1 revision) (flutter/flutter#137340)

...
CaseyHillers added a commit to flutter/devtools that referenced this pull request Oct 29, 2023
@CaseyHillers
Make SemanticsNode.isMergedIntoParent Readonly
74771df 
Follow up from flutter/flutter#137304
@CaseyHillers CaseyHillers mentioned this pull request Oct 29, 2023
Merged
7 tasks
@HansMuller HansMuller mentioned this pull request Nov 1, 2023
Open
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
@engine-flutter-autoroll @HugoOlthof
Roll Flutter from c555599fbb4a to 5907c9757e3f (45 revisions) (flutte…
9ea5493 
…r#5252)

Roll Flutter from c555599fbb4a to 5907c9757e3f (45 revisions)

flutter/flutter@c555599...5907c97

2023-10-27 gspencergoog@users.noreply.github.com Add `isLogicalKeyPressed` to `KeyEvent` (flutter/flutter#136856)
2023-10-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Ensure `flutter build apk --release` optimizes+shrinks platform code" (flutter/flutter#137433)
2023-10-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from bedc49efc85c to a198ad4e740d (20 revisions) (flutter/flutter#137429)
2023-10-27 31859944+LongCatIsLooong@users.noreply.github.com Make `SemanticsNode.isMergedIntoParent` Readonly (flutter/flutter#137304)
2023-10-27 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from bedc49efc85c to 71e1a0430232 (16 revisions) (flutter/flutter#137422)
2023-10-27 engine-flutter-autoroll@skia.org Roll Packages from fea24c5 to 2af6954 (5 revisions) (flutter/flutter#137421)
2023-10-27 15619084+vashworth@users.noreply.github.com Run tests on either macOS 12 or 13 (flutter/flutter#137365)
2023-10-27 tessertaha@gmail.com Revert "Update `DataTable` test when data row is pressed for Material 3 (#137230)" (flutter/flutter#137407)
2023-10-27 tessertaha@gmail.com Revert "Reland - Update `OutlinedButton` tests for Material 3 (#136809) (#137247)" (flutter/flutter#137406)
2023-10-27 kustermann@google.com Ensure `flutter build apk --release` optimizes+shrinks platform code (flutter/flutter#136880)
2023-10-27 tessertaha@gmail.com Update `DataTable` test when data row is pressed for Material 3 (flutter/flutter#137230)
2023-10-27 tessertaha@gmail.com Reland - Update `OutlinedButton` tests for Material 3 (#136809) (flutter/flutter#137247)
2023-10-27 andrewrkolos@gmail.com update asset manifest file name referenced in `WebServiceWorker` (flutter/flutter#135954)
2023-10-27 andrewrkolos@gmail.com give `throwsToolExit` a more useful description (flutter/flutter#136694)
2023-10-27 tvolkert@users.noreply.github.com Add ConstrainedLayoutBuilder.updateShouldRebuild() (flutter/flutter#136691)
2023-10-27 41873024+droidbg@users.noreply.github.com Fix. typos (flutter/flutter#137325)
2023-10-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 87f384c2d70b to bedc49efc85c (1 revision) (flutter/flutter#137382)
2023-10-26 pateltirth454@gmail.com Fix Typos (flutter/flutter#137292)
2023-10-26 sokolovskyi.konstantin@gmail.com AnimationController should dispatch creation in constructor. (flutter/flutter#134839)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9788bb9ff83e to 87f384c2d70b (4 revisions) (flutter/flutter#137380)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from ce1c1ee54107 to 9788bb9ff83e (3 revisions) (flutter/flutter#137373)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 windows_startup_test to be unflaky (flutter/flutter#137228)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 flutter_tool_startup__windows to be unflaky (flutter/flutter#137229)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 windows_home_scroll_perf__timeline_summary to be unflaky (flutter/flutter#137221)
2023-10-26 34871572+gmackall@users.noreply.github.com Declare dependency on copyFlutterAssetsTask in bundleAarTask (flutter/flutter#137370)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 complex_layout_win_desktop__start_up to be unflaky (flutter/flutter#137225)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 394744d2c4d0 to ce1c1ee54107 (2 revisions) (flutter/flutter#137367)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 hot_mode_dev_cycle_win_target__benchmark to be unflaky (flutter/flutter#137217)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 flutter_view_win_desktop__start_up to be unflaky (flutter/flutter#137226)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 platform_view_win_desktop__start_up to be unflaky (flutter/flutter#137227)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 flutter_gallery_win_desktop__start_up to be unflaky (flutter/flutter#137224)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 hello_world_win_desktop__compile to be unflaky (flutter/flutter#137222)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 flutter_gallery_win_desktop__compile to be unflaky (flutter/flutter#137223)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 run_release_test_windows to be unflaky (flutter/flutter#137220)
2023-10-26 greg@zulip.com Fix dislocated doc and comment on ThemeData localize cache (flutter/flutter#137315)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 platform_channel_sample_test_windows to be unflaky (flutter/flutter#137218)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7c5c8f587992 to 394744d2c4d0 (1 revision) (flutter/flutter#137364)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9363fe6ba503 to 7c5c8f587992 (3 revisions) (flutter/flutter#137363)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 542f8bc4c019 to 9363fe6ba503 (2 revisions) (flutter/flutter#137354)
2023-10-26 42216813+eliasyishak@users.noreply.github.com Unified analytics events for doctor validators (flutter/flutter#136647)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from d8132d5070bf to 542f8bc4c019 (2 revisions) (flutter/flutter#137349)
2023-10-26 fluttergithubbot@gmail.com Marks Windows_arm64 run_debug_test_windows to be unflaky (flutter/flutter#137219)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0a6253dbfafd to d8132d5070bf (1 revision) (flutter/flutter#137347)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5da115661f01 to 0a6253dbfafd (1 revision) (flutter/flutter#137341)
2023-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 61ae5ef94e9c to 5da115661f01 (1 revision) (flutter/flutter#137340)

...
richardshiue added a commit to richardshiue/modal_bottom_sheet that referenced this pull request Feb 5, 2024
@richardshiue
fix: adapt scrollable.dart to upstream SemanticsNode changes
8ee20bd 
This PR flutter/flutter#137304 makes SemanticsNode.isMergedIntoParent readonly, which makes the customized scrollable.dart incompatible.
@richardshiue richardshiue mentioned this pull request Feb 5, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
@engine-flutter-autoroll
7a3135b9c Make SemanticsNode.isMergedIntoParent Readonly (flutter/f…
6609fea 
…lutter#137304)
joscha0 added a commit to joscha0/flutter_sheet that referenced this pull request Feb 19, 2024
@joscha0
fix readonly isMergedIntoParent flutter/flutter#137304
0b0ee57
zikwall added a commit to freeorgan/sheet that referenced this pull request Feb 27, 2024
@zikwall
Fix: flutter v3.19
53c2173 
This PR flutter/flutter#137304 makes SemanticsNode.isMergedIntoParent readonly, which makes the customized scrollable.dart incompatible when using flutter version 3.18 and upwards
jamesblasco pushed a commit to jamesblasco/modal_bottom_sheet that referenced this pull request Mar 12, 2024
@richardshiue
fix: adapt scrollable.dart to upstream SemanticsNode changes (#387)
2703e68 
This PR flutter/flutter#137304 makes SemanticsNode.isMergedIntoParent readonly, which makes the customized scrollable.dart incompatible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goderbauer goderbauer goderbauer approved these changes

Assignees
No one assigned
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) autosubmit Merge PR when tree becomes green via auto submit App f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TextSpan with recognizer in ListTile fails assertion
2 participants
@LongCatIsLooong @goderbauer