Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Apr 28, 2021

Fix widget traversal when using the TAB key while a11y is enabled.

  • Synchronize editing state that's reported through the semantics tree as opposed to that coming through the flutter/textinput channel.
  • Add tabindex=0 onto tappable targets, otherwise the browser doesn't treat the element as focusable.

This is an alternative approach to the previously proposed #25401. This approach does not require changes on the framework side.

Fixes flutter/flutter#52106

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Apr 28, 2021
@google-cla google-cla bot added the cla: yes label Apr 28, 2021
@yjbanov yjbanov force-pushed the fix-a11y-tab-traversal-textSelection branch 3 times, most recently from 2572346 to afec643 Compare April 28, 2021 04:19
@yjbanov yjbanov force-pushed the fix-a11y-tab-traversal-textSelection branch from afec643 to 13f0431 Compare April 28, 2021 21:47
@yjbanov yjbanov requested review from ferhatb and mdebbar April 28, 2021 22:59
@yjbanov yjbanov marked this pull request as ready for review April 28, 2021 22:59
@yjbanov
Copy link
Contributor Author

yjbanov commented Apr 28, 2021

I'm going to debug the Mac Safari test but otherwise this is ready for review. I don't think the approach will change much.

/// https://developer.mozilla.org/en-US/docs/Web/API/Document/hasFocus
bool? get windowHasFocus =>
bool get windowHasFocus =>
js_util.callMethod(html.document, 'hasFocus', <dynamic>[]);
Copy link
Contributor

Choose a reason for hiding this comment

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

i would add ?? false here if there are browsers that don't implement and we have undefined return.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

void update() {
final html.Element element = semanticsObject.element;

// "tab-index" HTML attribute is necessary to make the element can be
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe reword: tab-index = 0 is used for allow keyboard traversal of non-form elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

_stopListening();
}
}
if (semanticsObject.isFlagsDirty && semanticsObject.hasFocus) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add comment on why we are setting focus here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

_subscriptions.clear();
_lastEditingState = null;

// If focused element is a part of a form, it needs to stay on the DOM
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this was added as a fix for form autofill, does disabling keep form submit intact?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I removed form autofill from semantics. It was broken. The non-a11y version is still intact.

// Element placement is done by [TextField].
}

@override
Copy link
Contributor

Choose a reason for hiding this comment

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

remove empty override?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The empty override makes these methods noop.

testInputElement = null;
});

test('autofill form lifecycle works', () async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an integration test for autofill that still covers this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not changing tests for non-a11y form autofill. This is a11y autofill, which I'm removing due to broken design.

@yjbanov yjbanov force-pushed the fix-a11y-tab-traversal-textSelection branch from 13f0431 to 6561387 Compare May 3, 2021 19:28
@yjbanov yjbanov requested a review from ferhatb May 3, 2021 19:28
@yjbanov yjbanov force-pushed the fix-a11y-tab-traversal-textSelection branch from 6561387 to 461af70 Compare May 7, 2021 18:33
@yjbanov yjbanov force-pushed the fix-a11y-tab-traversal-textSelection branch from 461af70 to 322fe17 Compare May 10, 2021 17:00
@yjbanov yjbanov force-pushed the fix-a11y-tab-traversal-textSelection branch from f5cf6eb to 6e942e6 Compare May 10, 2021 20:59
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 11, 2021
naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request May 21, 2021
* fix race with framework when using tab to traverse in a11y mode
christopherfujino added a commit that referenced this pull request May 26, 2021
* 'Update Dart SDK to 375a2d7'

* Fix a11y tab traversal (#25797)

* fix race with framework when using tab to traverse in a11y mode

* Fix CanvasKit SVG clipPath leak (#26227)

Co-authored-by: Yegor <yjbanov@google.com>
Co-authored-by: Harry Terkelsen <hterkelsen@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[WEB]: Accessibility focus border doesn't follow when navigating through interactive elements with tab key

2 participants