Allow Developers to enable Accessibility testing on WebFlutterDriver and get the underlying webDriver#65051
Conversation
| /// Enables accessibility feature. | ||
| Future<void> enableAccessibility() async { | ||
| throw UnimplementedError(); | ||
| } |
There was a problem hiding this comment.
Shall we add a message to the error?
There was a problem hiding this comment.
Discussed offline. Will follow existing pattern and do not add message to UnimplementedError.
| VMServiceClient get serviceClient => throw UnimplementedError(); | ||
|
|
||
| /// Getter of webDriver. | ||
| async_io.WebDriver get webDriver => throw UnimplementedError(); |
There was a problem hiding this comment.
Shall we add a message to the error here as well? Also a comment on why we add this method. (Sorry the serviceClient also doesn't have it, but I thought it might be useful in the future to the readers of this code)
There was a problem hiding this comment.
See the comment below.
| 'document.querySelector(\'flt-semantics-placeholder\').click();', | ||
| <String>[]); | ||
| _accessibilityEnabled = true; | ||
| } |
There was a problem hiding this comment.
There are a few things we need to do here.
(1) This only works for desktop web (mobile web has another method which is very similar to this)
(2) If flt-semantics-placeholder which lives on the engine changes, this will break. It would be very hard to understand for the engine-developers. It is best to add a documentation and also a link to that code.
Btw, a question we can also click on this button by calling the following right?
webDriver.execute(
'document.querySelector(\'flt-semantics-placeholder\').click();',
<String>[]);
There was a problem hiding this comment.
Updated accordingly. Well create an issue for Mobile Web (in fact, it works on Android-Chrome).
| String url, | ||
| Map<String, dynamic> settings, | ||
| {Duration timeout}) async { | ||
| // Use sync WebDriver because async version will create a 15 seconds |
There was a problem hiding this comment.
Question: Looks like we are also switching the way we are accessing the driver object (fromExistingSession vs constructor)? Is there a specific reason for it (anything failing with a11y or screendriver)?
There was a problem hiding this comment.
The async version of fromExistingSession returns a WebDriver instance which will NOT have the same capabilities (in fact, it would be empty) as the original one where the capabilities plays an important role for screendriver.
|
Can we add a test to this functionality? I think the easiest way would be editing this code: https://github.com/flutter/flutter/blob/master/examples/hello_world/test_driver/smoke_web_engine_test.dart we can add one more test method, which call enabling a11y. Later the webdriver can be called a second time to show, placeholder no longer exists and |
| // https://github.com/flutter/engine/blob/master/lib/web_ui/lib/src/engine/semantics/semantics.dart#L534 | ||
| final WebElement element = await driver.webDriver.findElement(const By.tagName('flt-semantics')); | ||
|
|
||
| expect(element, isNotNull); |
There was a problem hiding this comment.
Great, thanks for adding this test!
| test('enable accessibility', () async { | ||
| await driver.enableAccessibility(); | ||
|
|
||
| await Future<void>.delayed(const Duration(seconds: 2)); |
There was a problem hiding this comment.
Is there any way we can remove this timeout? In general this kind of thing is a source of flakes, and thus violates our style guide (https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#never-check-if-a-port-is-available-before-using-it-never-add-timeouts-and-other-race-conditions).
Description
Allow Developers to enable Accessibility testing on WebFlutterDriver and get the underlying webDriver.
Tests
I added the following tests:
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.