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

Don't crash if no root semantics node #2784

Merged
merged 1 commit into from
Jul 8, 2016
Merged

Don't crash if no root semantics node #2784

merged 1 commit into from
Jul 8, 2016

Conversation

abarth
Copy link
Contributor

@abarth abarth commented Jul 8, 2016

Fixes #4849

@abarth
Copy link
Contributor Author

abarth commented Jul 8, 2016

@tvolkert

tvolkert
tvolkert reviewed Jul 8, 2016
View reviewed changes
sky/shell/platform/ios/framework/Source/accessibility_bridge.mm
@@ -315,8 +315,9 @@ - (BOOL)accessibilityPerformMagicTap {
}
}

if (!view_.accessibilityElements) {
view_.accessibilityElements = @[ objects_[kRootNodeId] ];
SemanticObject* root = objects_[kRootNodeId];
Copy link
Contributor

Choose a reason for hiding this comment

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

Under what conditions do we receive an initial update (i.e. not having set view_.accessibilityElements yet) not containing root?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suppose someone empties out the render tree and there's nothing on screen.

@abarth
Copy link
Contributor Author

abarth commented Jul 8, 2016

Updated to clear out the list of accessible elements if there is no root element.

tvolkert
tvolkert reviewed Jul 8, 2016
View reviewed changes
sky/shell/platform/ios/framework/Source/accessibility_bridge.mm
view_.accessibilityElements = @[ objects_[kRootNodeId] ];
SemanticObject* root = objects_[kRootNodeId];
if (root) {
if (!view_.accessibilityElements && root) {
Copy link
Contributor

Choose a reason for hiding this comment

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

s/ && root//

@tvolkert
Copy link
Contributor

tvolkert commented Jul 8, 2016

lgtm

@abarth abarth merged commit 304881d into flutter:master Jul 8, 2016
@abarth abarth deleted the handle_null_root branch July 8, 2016 20:39
rhencke pushed a commit to rhencke/engine that referenced this pull request Dec 20, 2020
@jonasfj
Roll pub
41381e9 
a7e448b pub upgrade --major-versions (flutter#2784)
06bcff0 pub upgrade foo, only unlocks foo from pubspec.lock (flutter#2781)
aa20a4b Usage exceptions no stacktrace (flutter#2776)
14e12bf Clarify error message when spawning a subprocess fails (flutter#2763)
5ef0a0e Added a pub upgrade --nullsafety mode (flutter#2741)
9cde240 Fix pub remove --dry-run (flutter#2774)
7426be9 Remove hack now issue has been fixed (flutter#2773)
90a1f77 Limit retries in test to avoid timeout (flutter#2765)
3aa327c Stop depending on .packages (flutter#2764)

Change-Id: Ifd8a8402c4628e7278b6fedfd2ca7902004fa6e8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175241
Auto-Submit: Jonas Jensen <jonasfj@google.com>
Reviewed-by: Sigurd Meldgaard <sigurdm@google.com>
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
rhencke pushed a commit to rhencke/engine that referenced this pull request Dec 20, 2020
@davidmorgan
Revert "Roll pub"
c0e13c1 
This reverts commit 41381e9.

Reason for revert: pub fails on startup if git is not installed; b/175080640

Original change's description:
> Roll pub
>
> a7e448b pub upgrade --major-versions (flutter#2784)
> 06bcff0 pub upgrade foo, only unlocks foo from pubspec.lock (flutter#2781)
> aa20a4b Usage exceptions no stacktrace (flutter#2776)
> 14e12bf Clarify error message when spawning a subprocess fails (flutter#2763)
> 5ef0a0e Added a pub upgrade --nullsafety mode (flutter#2741)
> 9cde240 Fix pub remove --dry-run (flutter#2774)
> 7426be9 Remove hack now issue has been fixed (flutter#2773)
> 90a1f77 Limit retries in test to avoid timeout (flutter#2765)
> 3aa327c Stop depending on .packages (flutter#2764)
>
> Change-Id: Ifd8a8402c4628e7278b6fedfd2ca7902004fa6e8
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175241
> Auto-Submit: Jonas Jensen <jonasfj@google.com>
> Reviewed-by: Sigurd Meldgaard <sigurdm@google.com>
> Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>

TBR=sigurdm@google.com,jonasfj@google.com

Change-Id: I29b20b4a2dc037146dd27c8fe6e6ecce6833e7b6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175303
Reviewed-by: David Morgan <davidmorgan@google.com>
Commit-Queue: David Morgan <davidmorgan@google.com>
rhencke pushed a commit to rhencke/engine that referenced this pull request Dec 20, 2020
@sigurdm
Reland "Roll pub"
bf26eb3 
This is a reland of 41381e9

Contains one more fix:
8ea9612 fix tryGitCommand

Original change's description:
> Roll pub
>
> a7e448b pub upgrade --major-versions (flutter#2784)
> 06bcff0 pub upgrade foo, only unlocks foo from pubspec.lock (flutter#2781)
> aa20a4b Usage exceptions no stacktrace (flutter#2776)
> 14e12bf Clarify error message when spawning a subprocess fails (flutter#2763)
> 5ef0a0e Added a pub upgrade --nullsafety mode (flutter#2741)
> 9cde240 Fix pub remove --dry-run (flutter#2774)
> 7426be9 Remove hack now issue has been fixed (flutter#2773)
> 90a1f77 Limit retries in test to avoid timeout (flutter#2765)
> 3aa327c Stop depending on .packages (flutter#2764)
>
> Change-Id: Ifd8a8402c4628e7278b6fedfd2ca7902004fa6e8
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175241
> Auto-Submit: Jonas Jensen <jonasfj@google.com>
> Reviewed-by: Sigurd Meldgaard <sigurdm@google.com>
> Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>

Change-Id: Id50e6eca0372860f7087882a5c67141def07031c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175309
Reviewed-by: Jonas Jensen <jonasfj@google.com>
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
rhencke pushed a commit to rhencke/engine that referenced this pull request Dec 20, 2020
@jonasfj @walnutdust @sigurdm
pub upgrade --major-versions (flutter#2784)
a7e448b 
* Naive implementation

* Added test, works for dev_dependencies

* Changes per review comments

* Only remove upper bounds

* Function renaming

* Documentation

* Updated documentation

* Fix upgrade --breaking tests

* clean up argument parsing

* Improved dry-run output

* Fixed tests

* Removed unnecessary alias

* Update lib/src/command/upgrade.dart

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>

* Update lib/src/command/upgrade.dart

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>

* Update lib/src/command/upgrade.dart

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>

* Update lib/src/command/upgrade.dart

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>

* Update lib/src/command/upgrade.dart

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>

* Update lib/src/command/upgrade.dart

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>

* Update lib/src/command/upgrade.dart

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>

* Added warning for dependency overrides

* nit: fix comment

* Nits and tests for warning

Co-authored-by: Garett Tok <garettok@hotmail.com>
Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@abarth @tvolkert