-
Notifications
You must be signed in to change notification settings - Fork 3
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.
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
Add support for Apple Safari and VoiceOver #50
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Left an inline question below.
if (settings !== 'defaultMode') { | ||
throw new Error(`Unrecognized setting for VoiceOver: ${settings}`); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to enable these settings via AppleScript?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! I just checked the entry for VoiceOver in the macOS Script Editor "Library" (I'd link to it here, but I can't find an online reference), but there doesn't appear to be any API related to modifying settings.
This will be a problem if/when the CG wishes to write tests for behavior when one of these settings does not have its default value. As I understand things, though, the CG is currently limiting their scope to the screen reader's default settings. If that's the case, then lack of scripting support should be fine for now.
Can you confirm, @howard-e?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand things, though, the CG is currently limiting their scope to the screen reader's default settings.
Well not as of recently. There are now 28 test instances where default mode isn't being used with VO. This isn't in all test suites as yet though. I ran the following command in w3c/aria-at:
find tests -type f -name "voiceover_macos-commands.csv" -exec awk -F',' '$3 != null && $3 != "settings" {print FILENAME ":" $0}' {} \;
28 results
tests/link-span-text/data/voiceover_macos-commands.csv:navForwardsToLink,l ,singleQuickKeyNavOn,,3.2
tests/link-span-text/data/voiceover_macos-commands.csv:navBackToLink,shift+l,singleQuickKeyNavOn,,6.2
tests/command-button/data/voiceover_macos-commands.csv:navForwardsToButton,b,singleQuickKeyNavOn,,5.3
tests/command-button/data/voiceover_macos-commands.csv:navForwardsToButton,j ,singleQuickKeyNavOn,,5.2
tests/command-button/data/voiceover_macos-commands.csv:navBackToButton,shift+b,singleQuickKeyNavOn,,6.3
tests/command-button/data/voiceover_macos-commands.csv:navBackToButton,shift+j,singleQuickKeyNavOn,,6.2
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navForwardsInToRadioGroupWhereNoRadioButtonsAreChecked,j,singleQuickKeyNavOn,,2.2
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navBackInToRadioGroupWhereNoRadioButtonsAreChecked,shift+j,singleQuickKeyNavOn,,4.1
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navForwardsInToRadioGroupWhereFirstRadioButtonIsChecked,j,singleQuickKeyNavOn,,6.2
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:NavBackIntoRadioGroupWhereLastRadioChecked,shift+j,singleQuickKeyNavOn,,8.1
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navOutStartRadioGroup,shift+l,singleQuickKeyNavOn,3:groupBoundary ,10.2
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navOutEndRadioGroup,l,singleQuickKeyNavOn,3:groupBoundary ,12.2
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navToNextUncheckedRadio,j,singleQuickKeyNavOn,,20.1
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navToPrevUncheckedRadio,shift+j,singleQuickKeyNavOn,,22.1
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navToNextCheckedRadio,j,singleQuickKeyNavOn,,24.1
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navToPrevCheckedRadio,shift+j,singleQuickKeyNavOn,,26.1
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navToFirstRadio,down,arrowQuickKeyNavOff,,30
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navToFirstRadio,right,arrowQuickKeyNavOff,,30.1
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navToLastRadio,up,arrowQuickKeyNavOff,,32
tests/radiogroup-aria-activedescendant/data/voiceover_macos-commands.csv:navToLastRadio,left,arrowQuickKeyNavOff,,32.1
tests/toggle-button/data/voiceover_macos-commands.csv:navForwardsToNotPressedToggleButton,b,singleQuickKeyNavOn,,5.3
tests/toggle-button/data/voiceover_macos-commands.csv:navForwardsToNotPressedToggleButton,j ,singleQuickKeyNavOn,,5.2
tests/toggle-button/data/voiceover_macos-commands.csv:navBackToNotPressedToggleButton,shift+b,singleQuickKeyNavOn,,6.3
tests/toggle-button/data/voiceover_macos-commands.csv:navBackToNotPressedToggleButton,shift+j ,singleQuickKeyNavOn,,6.2
tests/toggle-button/data/voiceover_macos-commands.csv:navForwardsToPressedToggleButton,b,singleQuickKeyNavOn,,11.3
tests/toggle-button/data/voiceover_macos-commands.csv:navForwardsToPressedToggleButton,j ,singleQuickKeyNavOn,,11.2
tests/toggle-button/data/voiceover_macos-commands.csv:navBackToPressedToggleButton,shift+b,singleQuickKeyNavOn,,12.3
tests/toggle-button/data/voiceover_macos-commands.csv:navBackToPressedToggleButton,shift+j ,singleQuickKeyNavOn,,12.2
If that's the case, then lack of scripting support should be fine for now.
Even though there are now instances, this should still be fine for the moment. It's certainly a current limitation that hasn't yet been explored, but considerations for this should now be reflected in the app as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'll make a note to bring this up at the next Community Group meeting. (It's too early to say that it's impossible to support this, but doing so will almost certainly take some non-trivial effort.)
Introduce an interface named "BrowserDriver" to minimize the impact of Safari automation (which cannot use WebDriver due to this project's specific needs).
(This patch is formatted as a series of commits in order to more precisely describe the changes; it is intended to be merged with a merge commit in order to retain that detail in the project history.)