Skip to content

Bring back paste button hide behavior 2#56922

Merged
justinmc merged 2 commits intoflutter:masterfrom
justinmc:pasteable-revert-revert-2
May 11, 2020
Merged

Bring back paste button hide behavior 2#56922
justinmc merged 2 commits intoflutter:masterfrom
justinmc:pasteable-revert-revert-2

Conversation

@justinmc
Copy link
Contributor

@justinmc justinmc commented May 11, 2020

Original PR that has been reverted twice now: #54902

This caused a failure where the ClipboardStatusNotifier was modified after being disposed, which seems to be caused by receiving the clipboard data after disposal. My one line fix for this is to check if it's disposed before updating the value.

@justinmc justinmc self-assigned this May 11, 2020
@fluttergithubbot fluttergithubbot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels May 11, 2020
@justinmc justinmc requested a review from goderbauer May 11, 2020 17:08
goderbauer
goderbauer approved these changes May 11, 2020
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

packages/flutter/lib/src/widgets/text_selection.dart Outdated
Copy link
Member

@goderbauer goderbauer May 11, 2020

Choose a reason for hiding this comment

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

Let's move the if (_disposed) check to be the first thing in this callback since we don't have to do any of the other work if that happens.

Copy link
Contributor Author

@justinmc justinmc May 11, 2020

Choose a reason for hiding this comment

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

Smart, will do.

@justinmc justinmc force-pushed the pasteable-revert-revert-2 branch from 8f79c5e to 0a2c0a5 Compare May 11, 2020 17:39
@justinmc justinmc merged commit d988226 into flutter:master May 11, 2020
@justinmc justinmc deleted the pasteable-revert-revert-2 branch May 11, 2020 20:23
@jmagman
Copy link
Member

jmagman commented May 11, 2020

I believe this causes a new failure:

2020-05-11T14:41:55.457819: stdout: [+1992 ms] 00:09 �[32m+0�[0m�[31m -1�[0m: AccessibilityBridge TextField TextField has correct Android semantics �[1m�[31m[E]�[0m�[0m
2020-05-11T14:41:55.457984: stdout: [        ]   Expected: AndroidSemanticsNode with className: android.widget.EditText with actions: [AndroidSemanticsAction.clearAccessibilityFocus, AndroidSemanticsAction.click, AndroidSemanticsAction.setSelection] with flag isEditable: true with flag isFocusable: true with flag isFocused: true with flag isPassword: false
2020-05-11T14:41:55.458259: stdout: [        ]     Actual: AndroidSemanticsNode:<{flags: {isEditable: true, isEnabled: true, isDismissable: false, isCheckable: false, isChecked: false, isFocused: true, isLongClickable: false, isFocusable: true, isPassword: false}, id: 14, text: null, rect: {height: 144, width: 1080, bottom: 128.0, top: 80.0, left: 0.0, right: 360.0}, actions: [131072, 32768, 16, 128], className: android.widget.EditText, liveRegion: 1, contentDescription: null}>
2020-05-11T14:41:55.458442: stdout: [        ]      Which: Expected actions: [AndroidSemanticsAction.clearAccessibilityFocus, AndroidSemanticsAction.click, AndroidSemanticsAction.setSelection]
2020-05-11T14:41:55.458710: stdout: [        ]             Actual actions: [AndroidSemanticsAction.clearAccessibilityFocus, AndroidSemanticsAction.click, AndroidSemanticsAction.copy, AndroidSemanticsAction.setSelection]
2020-05-11T14:41:55.458935: stdout: [        ]             Unexpected: {AndroidSemanticsAction.copy}
2020-05-11T14:41:55.459211: stdout: [        ]             Missing: {}

android_semantics_integration_test_d988226_2.log

jmagman added a commit that referenced this pull request May 11, 2020
@jmagman
Revert "Bring back paste button hide behavior 2 (#56922)"
c292e2c 
This reverts commit d988226.
@jmagman jmagman mentioned this pull request May 11, 2020
Merged
jmagman added a commit that referenced this pull request May 11, 2020
@jmagman
Revert "Bring back paste button hide behavior 2 (#56922)" (#56951)
c969b8a 
This reverts commit d988226.
justinmc added a commit to justinmc/flutter that referenced this pull request May 13, 2020
@justinmc
Revert "Revert "Bring back paste button hide behavior 2 (flutter#56922)…
2a5a2fb 
…" (flutter#56951)"

This reverts commit c969b8a.
@justinmc justinmc mentioned this pull request May 13, 2020
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@goderbauer goderbauer goderbauer approved these changes

Assignees

@justinmc justinmc

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) c: contributor-productivity Team-specific productivity, code health, technical debt. f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. 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.

5 participants

@justinmc @jmagman @goderbauer @googlebot @fluttergithubbot