Bug 1598060 - access accessibility walker front through accessible front wherever possible. r=rcaliman

Differential Revision: https://phabricator.services.mozilla.com/D54528

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Yura Zenevich 2019-11-26 19:03:11 +00:00
parent b7e64c314b
commit 5c47541698
8 changed files with 213 additions and 190 deletions

View File

@ -82,7 +82,6 @@ class AccessibilityRow extends Component {
...TreeRow.propTypes,
hasContextMenu: PropTypes.bool.isRequired,
dispatch: PropTypes.func.isRequired,
accessibilityWalker: PropTypes.object,
scrollContentNodeIntoView: PropTypes.bool.isRequired,
supports: PropTypes.object,
};
@ -94,7 +93,7 @@ class AccessibilityRow extends Component {
scrollContentNodeIntoView,
} = this.props;
if (selected) {
this.unhighlight();
this.unhighlight(object);
this.update();
this.highlight(
object,
@ -119,7 +118,7 @@ class AccessibilityRow extends Component {
} = this.props;
// If row is selected, update corresponding accessible details.
if (!prevProps.member.selected && selected) {
this.unhighlight();
this.unhighlight(object);
this.update();
this.highlight(
object,
@ -192,14 +191,14 @@ class AccessibilityRow extends Component {
* Promise that resolves when the node is scrolled into view if
* possible.
*/
async scrollNodeIntoViewIfNeeded(accessible) {
if (!accessible.actorID) {
async scrollNodeIntoViewIfNeeded(accessibleFront) {
if (!accessibleFront.actorID) {
return;
}
const domWalker = (await accessible.targetFront.getFront("inspector"))
const domWalker = (await accessibleFront.targetFront.getFront("inspector"))
.walker;
const node = await domWalker.getNodeFromActor(accessible.actorID, [
const node = await domWalker.getNodeFromActor(accessibleFront.actorID, [
"rawAccessible",
"DOMNode",
]);
@ -217,34 +216,40 @@ class AccessibilityRow extends Component {
}
}
async highlight(accessible, options, scrollContentNodeIntoView) {
const { accessibilityWalker, dispatch } = this.props;
dispatch(unhighlight());
async highlight(accessibleFront, options, scrollContentNodeIntoView) {
this.props.dispatch(unhighlight());
if (!accessibleFront) {
return;
}
if (!accessible || !accessibilityWalker) {
const accessibilityWalkerFront = accessibleFront.parent();
if (!accessibilityWalkerFront) {
return;
}
// If necessary scroll the node into view before showing the accessibility
// highlighter.
if (scrollContentNodeIntoView) {
await this.scrollNodeIntoViewIfNeeded(accessible);
await this.scrollNodeIntoViewIfNeeded(accessibleFront);
}
accessibilityWalker
.highlightAccessible(accessible, options)
accessibilityWalkerFront
.highlightAccessible(accessibleFront, options)
.catch(error => console.warn(error));
}
unhighlight() {
const { accessibilityWalker, dispatch } = this.props;
dispatch(unhighlight());
if (!accessibilityWalker) {
unhighlight(accessibleFront) {
this.props.dispatch(unhighlight());
if (!accessibleFront) {
return;
}
accessibilityWalker.unhighlight().catch(error => console.warn(error));
const accessibilityWalkerFront = accessibleFront.parent();
if (!accessibilityWalkerFront) {
return;
}
accessibilityWalkerFront.unhighlight().catch(error => console.warn(error));
}
async printToJSON() {
@ -306,13 +311,13 @@ class AccessibilityRow extends Component {
...this.props,
onContextMenu: this.props.hasContextMenu && (e => this.onContextMenu(e)),
onMouseOver: () => this.highlight(member.object),
onMouseOut: () => this.unhighlight(),
onMouseOut: () => this.unhighlight(member.object),
key: `${member.path}-${member.active ? "active" : "inactive"}`,
};
return AuditController(
{
accessible: member.object,
accessibleFront: member.object,
},
AuditFilter({}, HighlightableTreeRow(props))
);

View File

@ -48,7 +48,7 @@ class AccessibilityRowValue extends Component {
audit &&
AuditController(
{
accessible: member.object,
accessibleFront: member.object,
},
Badges()
)

View File

@ -94,12 +94,12 @@ class AccessibilityTree extends Component {
* Handle accessible reorder event. If the accessible is cached and rendered
* within the accessibility tree, re-fetch its children and re-render the
* corresponding subtree.
* @param {Object} accessible accessible object that had its subtree
* reordered.
* @param {Object} accessibleFront
* accessible front that had its subtree reordered.
*/
onReorder(accessible) {
if (this.props.accessibles.has(accessible.actorID)) {
this.props.dispatch(fetchChildren(accessible));
onReorder(accessibleFront) {
if (this.props.accessibles.has(accessibleFront.actorID)) {
this.props.dispatch(fetchChildren(accessibleFront));
}
}
@ -107,22 +107,23 @@ class AccessibilityTree extends Component {
* Handle accessible name change event. If the name of an accessible changes
* and that accessible is cached and rendered within the accessibility tree,
* re-fetch its parent's children and re-render the corresponding subtree.
* @param {Object} accessible accessible object that had its name changed.
* @param {Object} parent optional parent accessible object. Note: if it
* parent is not present, we assume that the top
* level document's name has changed and use
* accessible walker as a parent.
* @param {Object} accessibleFront
* accessible front that had its name changed.
* @param {Object} parentFront
* optional parent accessible front. Note: if it parent is not
* present, we assume that the top level document's name has changed
* and use accessible walker as a parent.
*/
onNameChange(accessible, parent) {
onNameChange(accessibleFront, parentFront) {
const { accessibles, dispatch } = this.props;
const accessibilityWalkerFront = accessible.parent();
parent = parent || accessibilityWalkerFront;
const accessibilityWalkerFront = accessibleFront.parent();
parentFront = parentFront || accessibilityWalkerFront;
if (
accessibles.has(accessible.actorID) ||
accessibles.has(parent.actorID)
accessibles.has(accessibleFront.actorID) ||
accessibles.has(parentFront.actorID)
) {
dispatch(fetchChildren(parent));
dispatch(fetchChildren(parentFront));
}
}
@ -131,13 +132,13 @@ class AccessibilityTree extends Component {
* an accessible changes and that accessible is cached and rendered within the
* accessibility tree, re-fetch its children and re-render the corresponding
* subtree.
* @param {Object} accessible accessible object that had its child text
* changed.
* @param {Object} accessibleFront
* accessible front that had its child text changed.
*/
onTextChange(accessible) {
onTextChange(accessibleFront) {
const { accessibles, dispatch } = this.props;
if (accessibles.has(accessible.actorID)) {
dispatch(fetchChildren(accessible));
if (accessibles.has(accessibleFront.actorID)) {
dispatch(fetchChildren(accessibleFront));
}
}
@ -180,7 +181,6 @@ class AccessibilityTree extends Component {
const highlighted = object === highlightedItem;
return AccessibilityRow(
Object.assign({}, rowProps, {
accessibilityWalker,
hasContextMenu,
highlighted,
decorator: {

View File

@ -58,19 +58,22 @@ const TREE_DEPTH_PADDING_INCREMENT = 20;
class AccessiblePropertyClass extends Component {
static get propTypes() {
return {
accessible: PropTypes.string,
accessibleFrontActorID: PropTypes.string,
object: PropTypes.any,
focused: PropTypes.bool,
children: PropTypes.func,
};
}
componentDidUpdate({ object: prevObject, accessible: prevAccessible }) {
const { accessible, object, focused } = this.props;
componentDidUpdate({
object: prevObject,
accessibleFrontActorID: prevAccessibleFrontActorID,
}) {
const { accessibleFrontActorID, object, focused } = this.props;
// Fast check if row is focused or if the value did not update.
if (
focused ||
accessible !== prevAccessible ||
accessibleFrontActorID !== prevAccessibleFrontActorID ||
prevObject === object ||
(object && prevObject && typeof object === "object")
) {
@ -102,15 +105,14 @@ const AccessibleProperty = createFactory(AccessiblePropertyClass);
class Accessible extends Component {
static get propTypes() {
return {
accessible: PropTypes.object,
accessibleFront: PropTypes.object,
dispatch: PropTypes.func.isRequired,
DOMNode: PropTypes.object,
nodeFront: PropTypes.object,
items: PropTypes.array,
labelledby: PropTypes.string.isRequired,
parents: PropTypes.object,
relations: PropTypes.object,
supports: PropTypes.object,
accessibilityWalker: PropTypes.object.isRequired,
};
}
@ -135,18 +137,25 @@ class Accessible extends Component {
);
}
componentWillReceiveProps({ accessible }) {
const oldAccessible = this.props.accessible;
componentWillReceiveProps({ accessibleFront }) {
const oldAccessibleFront = this.props.accessibleFront;
if (oldAccessible) {
if (accessible && accessible.actorID === oldAccessible.actorID) {
if (oldAccessibleFront) {
if (
accessibleFront &&
accessibleFront.actorID === oldAccessibleFront.actorID
) {
return;
}
ACCESSIBLE_EVENTS.forEach(event => oldAccessible.off(event, this.update));
ACCESSIBLE_EVENTS.forEach(event =>
oldAccessibleFront.off(event, this.update)
);
}
if (accessible) {
ACCESSIBLE_EVENTS.forEach(event => accessible.on(event, this.update));
if (accessibleFront) {
ACCESSIBLE_EVENTS.forEach(event =>
accessibleFront.on(event, this.update)
);
}
}
@ -156,9 +165,11 @@ class Accessible extends Component {
this.onAccessibleInspected
);
const { accessible } = this.props;
if (accessible) {
ACCESSIBLE_EVENTS.forEach(event => accessible.off(event, this.update));
const { accessibleFront } = this.props;
if (accessibleFront) {
ACCESSIBLE_EVENTS.forEach(event =>
accessibleFront.off(event, this.update)
);
}
}
@ -170,15 +181,15 @@ class Accessible extends Component {
}
async update() {
const { dispatch, accessible, supports } = this.props;
if (!accessible.actorID) {
const { dispatch, accessibleFront, supports } = this.props;
if (!accessibleFront.actorID) {
return;
}
const domWalker = (await accessible.targetFront.getFront("inspector"))
const domWalker = (await accessibleFront.targetFront.getFront("inspector"))
.walker;
dispatch(updateDetails(domWalker, accessible, supports));
dispatch(updateDetails(domWalker, accessibleFront, supports));
}
setExpanded(item, isExpanded) {
@ -211,34 +222,42 @@ class Accessible extends Component {
await highlighterFront.unhighlight();
}
showAccessibleHighlighter(accessible) {
const { accessibilityWalker, dispatch } = this.props;
dispatch(unhighlight());
if (!accessible || !accessibilityWalker) {
showAccessibleHighlighter(accessibleFront) {
this.props.dispatch(unhighlight());
if (!accessibleFront) {
return;
}
accessibilityWalker.highlightAccessible(accessible).catch(error => {
// Only report an error where there's still a toolbox. Ignore cases where toolbox is
// already destroyed.
if (gToolbox) {
console.error(error);
}
});
const accessibilityWalkerFront = accessibleFront.parent();
if (!accessibilityWalkerFront) {
return;
}
accessibilityWalkerFront
.highlightAccessible(accessibleFront)
.catch(error => {
// Only report an error where there's still a toolbox. Ignore cases
// where toolbox is already destroyed.
if (gToolbox) {
console.error(error);
}
});
}
hideAccessibleHighlighter() {
const { accessibilityWalker, dispatch } = this.props;
dispatch(unhighlight());
if (!accessibilityWalker) {
hideAccessibleHighlighter(accessibleFront) {
this.props.dispatch(unhighlight());
if (!accessibleFront) {
return;
}
accessibilityWalker.unhighlight().catch(error => {
// Only report an error where there's still a toolbox. Ignore cases where toolbox is
// already destroyed.
const accessibilityWalkerFront = accessibleFront.parent();
if (!accessibilityWalkerFront) {
return;
}
accessibilityWalkerFront.unhighlight().catch(error => {
// Only report an error where there's still a toolbox. Ignore cases where
// toolbox is already destroyed.
if (gToolbox) {
console.error(error);
}
@ -259,13 +278,19 @@ class Accessible extends Component {
.then(() => gToolbox.selection.setNodeFront(nodeFront, reason));
}
async selectAccessible(accessible) {
const { accessibilityWalker, dispatch } = this.props;
if (!accessibilityWalker) {
async selectAccessible(accessibleFront) {
if (!accessibleFront) {
return;
}
await dispatch(select(accessibilityWalker, accessible));
const accessibilityWalkerFront = accessibleFront.parent();
if (!accessibilityWalkerFront) {
return;
}
await this.props.dispatch(
select(accessibilityWalkerFront, accessibleFront)
);
const { props } = this.refs;
if (props) {
@ -289,17 +314,19 @@ class Accessible extends Component {
openLink: this.openLink,
};
if (isNode(object)) {
if (isNodeFront(object)) {
valueProps.defaultRep = ElementNode;
valueProps.onDOMNodeMouseOut = () =>
this.hideHighlighter(this.props.DOMNode);
this.hideHighlighter(this.props.nodeFront);
valueProps.onDOMNodeMouseOver = () =>
this.showHighlighter(this.props.DOMNode);
valueProps.onInspectIconClick = () => this.selectNode(this.props.DOMNode);
} else if (isAccessible(object)) {
this.showHighlighter(this.props.nodeFront);
valueProps.onInspectIconClick = () =>
this.selectNode(this.props.nodeFront);
} else if (isAccessibleFront(object)) {
const target = findAccessibleTarget(this.props.relations, object.actor);
valueProps.defaultRep = AccessibleRep;
valueProps.onAccessibleMouseOut = () => this.hideAccessibleHighlighter();
valueProps.onAccessibleMouseOut = () =>
this.hideAccessibleHighlighter(target);
valueProps.onAccessibleMouseOver = () =>
this.showAccessibleHighlighter(target);
valueProps.onInspectIconClick = (obj, e) => {
@ -321,7 +348,11 @@ class Accessible extends Component {
const depthPadding = depth * TREE_DEPTH_PADDING_INCREMENT;
return AccessibleProperty(
{ object, focused, accessible: this.props.accessible.actorID },
{
object,
focused,
accessibleFrontActorID: this.props.accessibleFront.actorID,
},
() =>
div(
{
@ -346,9 +377,9 @@ class Accessible extends Component {
render() {
const { expanded, active, focused } = this.state;
const { items, parents, accessible, labelledby } = this.props;
const { items, parents, accessibleFront, labelledby } = this.props;
if (accessible) {
if (accessibleFront) {
return Tree({
ref: "props",
key: "accessible-properties",
@ -430,18 +461,18 @@ const findByPath = (path, items) => {
};
/**
* Check if a given property is a DOMNode actor.
* Check if a given property is a DOMNode front.
* @param {Object?} value A property to check for being a DOMNode.
* @return {Boolean} A flag that indicates whether a property is a DOMNode.
*/
const isNode = value => value && value.typeName === "domnode";
const isNodeFront = value => value && value.typeName === "domnode";
/**
* Check if a given property is an Accessible actor.
* Check if a given property is an Accessible front.
* @param {Object?} value A property to check for being an Accessible.
* @return {Boolean} A flag that indicates whether a property is an Accessible.
*/
const isAccessible = value => value && value.typeName === "accessible";
const isAccessibleFront = value => value && value.typeName === "accessible";
/**
* While waiting for a reps fix in https://github.com/firefox-devtools/reps/issues/92,
@ -481,9 +512,9 @@ const makeItemsForDetails = (props, parentPath) =>
let contents = props[name];
if (contents) {
if (isNode(contents)) {
if (isNodeFront(contents)) {
contents = translateNodeFrontToGripWrapper(contents);
} else if (isAccessible(contents)) {
} else if (isAccessibleFront(contents)) {
contents = translateAccessibleFrontToGrip(contents);
} else if (Array.isArray(contents) || typeof contents === "object") {
children = makeItemsForDetails(contents, path);
@ -510,22 +541,26 @@ const makeParentMap = items => {
};
const mapStateToProps = ({ details, ui }) => {
const { accessible, DOMNode, relations } = details;
const {
accessible: accessibleFront,
DOMNode: nodeFront,
relations,
} = details;
const { supports } = ui;
if (!accessible || !DOMNode) {
if (!accessibleFront || !nodeFront) {
return {};
}
const items = makeItemsForDetails(
ORDERED_PROPS.reduce((props, key) => {
if (key === "DOMNode") {
props.DOMNode = DOMNode;
props.nodeFront = nodeFront;
} else if (key === "relations") {
if (supports.relations) {
props.relations = relations;
}
} else {
props[key] = accessible[key];
props[key] = accessibleFront[key];
}
return props;
@ -534,7 +569,7 @@ const mapStateToProps = ({ details, ui }) => {
);
const parents = makeParentMap(items);
return { accessible, DOMNode, items, parents, relations, supports };
return { accessibleFront, nodeFront, items, parents, relations, supports };
};
module.exports = connect(mapStateToProps)(Accessible);

View File

@ -10,7 +10,7 @@ const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
class AuditController extends React.Component {
static get propTypes() {
return {
accessible: PropTypes.object.isRequired,
accessibleFront: PropTypes.object.isRequired,
children: PropTypes.any,
};
}
@ -19,7 +19,7 @@ class AuditController extends React.Component {
super(props);
const {
accessible: { checks },
accessibleFront: { checks },
} = props;
this.state = {
checks,
@ -29,8 +29,8 @@ class AuditController extends React.Component {
}
componentWillMount() {
const { accessible } = this.props;
accessible.on("audited", this.onAudited);
const { accessibleFront } = this.props;
accessibleFront.on("audited", this.onAudited);
}
componentDidMount() {
@ -42,36 +42,36 @@ class AuditController extends React.Component {
}
componentWillUnmount() {
const { accessible } = this.props;
accessible.off("audited", this.onAudited);
const { accessibleFront } = this.props;
accessibleFront.off("audited", this.onAudited);
}
onAudited() {
const { accessible } = this.props;
if (!accessible.actorID) {
// Accessible is being removed, stop listening for 'audited' events.
accessible.off("audited", this.onAudited);
const { accessibleFront } = this.props;
if (!accessibleFront.actorID) {
// Accessible front is being removed, stop listening for 'audited' events.
accessibleFront.off("audited", this.onAudited);
return;
}
this.setState({ checks: accessible.checks });
this.setState({ checks: accessibleFront.checks });
}
maybeRequestAudit() {
const { accessible } = this.props;
if (!accessible.actorID) {
// Accessible is being removed, stop listening for 'audited' events.
accessible.off("audited", this.onAudited);
const { accessibleFront } = this.props;
if (!accessibleFront.actorID) {
// Accessible front is being removed, stop listening for 'audited' events.
accessibleFront.off("audited", this.onAudited);
return;
}
if (accessible.checks) {
if (accessibleFront.checks) {
return;
}
accessible.audit().catch(error => {
accessibleFront.audit().catch(error => {
// Accessible actor was destroyed, connection closed.
if (accessible.actorID) {
if (accessibleFront.actorID) {
console.warn(error);
}
});

View File

@ -154,7 +154,7 @@ class MainFrame extends Component {
},
AccessibilityTree({ accessibilityWalker })
),
endPanel: RightSidebar({ accessibilityWalker }),
endPanel: RightSidebar(),
vert: this.useLandscapeMode,
})
)

View File

@ -4,11 +4,7 @@
"use strict";
// React
const {
Component,
createFactory,
} = require("devtools/client/shared/vendor/react");
const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
const { createFactory } = require("devtools/client/shared/vendor/react");
const { div } = require("devtools/client/shared/vendor/react-dom-factories");
const { L10N } = require("../utils/l10n");
@ -19,51 +15,38 @@ const Accordion = createFactory(
const Checks = createFactory(require("./Checks"));
// Component that is responsible for rendering accessible panel's sidebar.
class RightSidebar extends Component {
static get propTypes() {
return {
accessibilityWalker: PropTypes.object.isRequired,
};
}
function RightSidebar() {
const propertiesID = "accessibility-properties";
const checksID = "accessibility-checks";
/**
* Render the sidebar component.
* @returns Sidebar React component.
*/
render() {
const propertiesID = "accessibility-properties";
const checksID = "accessibility-checks";
const { accessibilityWalker } = this.props;
return div(
{
className: "right-sidebar",
role: "presentation",
},
Accordion({
items: [
{
className: "checks",
component: Checks,
componentProps: { labelledby: `${checksID}-header` },
header: L10N.getStr("accessibility.checks"),
id: checksID,
opened: true,
return div(
{
className: "right-sidebar",
role: "presentation",
},
Accordion({
items: [
{
className: "checks",
component: Checks,
componentProps: { labelledby: `${checksID}-header` },
header: L10N.getStr("accessibility.checks"),
id: checksID,
opened: true,
},
{
className: "accessible",
component: Accessible,
componentProps: {
labelledby: `${propertiesID}-header`,
},
{
className: "accessible",
component: Accessible,
componentProps: {
accessibilityWalker,
labelledby: `${propertiesID}-header`,
},
header: L10N.getStr("accessibility.properties"),
id: propertiesID,
opened: true,
},
],
})
);
}
header: L10N.getStr("accessibility.properties"),
id: propertiesID,
opened: true,
},
],
})
);
}
module.exports = RightSidebar;

View File

@ -17,11 +17,11 @@ const {
describe("AuditController component:", () => {
it("dead accessible actor", () => {
const accessible = mockAccessible();
const accessibleFront = mockAccessible();
const wrapper = mount(
AuditController(
{
accessible,
accessibleFront,
},
span()
)
@ -39,47 +39,47 @@ describe("AuditController component:", () => {
});
const instance = wrapper.instance();
expect(accessible.on.mock.calls.length).toBe(1);
expect(accessible.off.mock.calls.length).toBe(1);
expect(accessible.on.mock.calls[0]).toEqual([
expect(accessibleFront.on.mock.calls.length).toBe(1);
expect(accessibleFront.off.mock.calls.length).toBe(1);
expect(accessibleFront.on.mock.calls[0]).toEqual([
"audited",
instance.onAudited,
]);
expect(accessible.off.mock.calls[0]).toEqual([
expect(accessibleFront.off.mock.calls[0]).toEqual([
"audited",
instance.onAudited,
]);
});
it("accessible without checks", () => {
const accessible = mockAccessible({
const accessibleFront = mockAccessible({
actorID: "1",
});
const wrapper = mount(
AuditController(
{
accessible,
accessibleFront,
},
span()
)
);
expect(wrapper.html()).toMatchSnapshot();
expect(accessible.audit.mock.calls.length).toBe(1);
expect(accessible.on.mock.calls.length).toBe(1);
expect(accessible.off.mock.calls.length).toBe(0);
expect(accessibleFront.audit.mock.calls.length).toBe(1);
expect(accessibleFront.on.mock.calls.length).toBe(1);
expect(accessibleFront.off.mock.calls.length).toBe(0);
});
it("accessible with checks", () => {
const checks = { foo: "bar" };
const accessible = mockAccessible({
const accessibleFront = mockAccessible({
actorID: "1",
checks,
});
const wrapper = mount(
AuditController(
{
accessible,
accessibleFront,
},
span({ className: "child" })
)