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

fix: revise interesting/uninteresting classification for AXNodes #6334

Merged
merged 1 commit into from Aug 14, 2020
Merged

fix: revise interesting/uninteresting classification for AXNodes #6334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion new-docs/puppeteer.snapshotoptions.interestingonly.md
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@

## SnapshotOptions.interestingOnly property

Prune unintersting nodes from the tree.
Prune uninteresting nodes from the tree.

<b>Signature:</b>

Expand Down
4 changes: 2 additions & 2 deletions new-docs/puppeteer.snapshotoptions.md
View file
Open in desktop
Expand Up @@ -15,6 +15,6 @@ export interface SnapshotOptions

| Property | Type | Description |
| --- | --- | --- |
| [interestingOnly](./puppeteer.snapshotoptions.interestingonly.md) | boolean | Prune unintersting nodes from the tree. |
| [root](./puppeteer.snapshotoptions.root.md) | [ElementHandle](./puppeteer.elementhandle.md) | Prune unintersting nodes from the tree. |
| [interestingOnly](./puppeteer.snapshotoptions.interestingonly.md) | boolean | Prune uninteresting nodes from the tree. |
| [root](./puppeteer.snapshotoptions.root.md) | [ElementHandle](./puppeteer.elementhandle.md) | Root node to get the accessibility tree for |

2 changes: 1 addition & 1 deletion new-docs/puppeteer.snapshotoptions.root.md
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@

## SnapshotOptions.root property

Prune unintersting nodes from the tree.
Root node to get the accessibility tree for

<b>Signature:</b>

Expand Down
15 changes: 7 additions & 8 deletions src/common/Accessibility.ts
View file
Open in desktop
Expand Up @@ -96,12 +96,12 @@ export interface SerializedAXNode {
*/
export interface SnapshotOptions {
/**
* Prune unintersting nodes from the tree.
* Prune uninteresting nodes from the tree.
* @defaultValue true
*/
interestingOnly?: boolean;
/**
* Prune unintersting nodes from the tree.
* Root node to get the accessibility tree for
* @defaultValue The root node of the entire page.
*/
root?: ElementHandle;
Expand Down Expand Up @@ -244,12 +244,14 @@ class AXNode {
private _hidden = false;
private _name: string;
private _role: string;
private _ignored: boolean;
private _cachedHasFocusableChild?: boolean;

constructor(payload: Protocol.Accessibility.AXNode) {
this.payload = payload;
this._name = this.payload.name ? this.payload.name.value : '';
this._role = this.payload.role ? this.payload.role.value : 'Unknown';
this._ignored = this.payload.ignored;

for (const property of this.payload.properties || []) {
if (property.name === 'editable') {
Expand All @@ -264,11 +266,7 @@ class AXNode {
private _isPlainTextField(): boolean {
if (this._richlyEditable) return false;
if (this._editable) return true;
return (
this._role === 'textbox' ||
this._role === 'ComboBox' ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, what was this 'ComboBox' role and why is it not relevant anymore?

Copy link
Collaborator Author

@johanbay johanbay Aug 14, 2020 •
edited

Choose a reason for hiding this comment

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

I'm definitely not very knowledgeable in the area, but from searching around I concluded that there is no such role in the spec. There is a combobox, but I'm not sure we want to categorize those as plain text fields? At least our tests don't want that. I think we should remove the ComboBox for now as it is confusing.

this._role === 'searchbox'
);
return this._role === 'textbox' || this._role === 'searchbox';
}

private _isTextOnlyObject(): boolean {
Expand Down Expand Up @@ -354,6 +352,7 @@ class AXNode {
case 'tab':
case 'textbox':
case 'tree':
case 'treeitem':
return true;
default:
return false;
Expand All @@ -362,7 +361,7 @@ class AXNode {

public isInteresting(insideControl: boolean): boolean {
const role = this._role;
if (role === 'Ignored' || this._hidden) return false;
if (role === 'Ignored' || this._hidden || this._ignored) return false;

if (this._focusable || this._richlyEditable) return true;

Expand Down