1806356 - [CTW] Transforms are not applied if they are attached to an element with no Accessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

STR:

1. Open this test case:
   data:text/html,<div style="translate: 0 300px;"><p>hi
2. Check the y coordinate for the paragraph.
   • Expected: It should account for the transform.
   • Actual: It doesn't account for the transform.

This happens because the div doesn't get an Accessible. If you add role="main" to the div, this works as expected.

Note that even on an Accessible that otherwise would have been created, role="presentation" would prevent that, which would also cause this problem.

I guess we need to ensure that elements with possible transforms get Accessibles. We can probably do this in nsAccessibilityService::MustBeAccessible by checking nsIFrame::IsTransformed.

Comment 2

[James Teh [:Jamie]]

Nathan, do you want to take a stab at this? See my implementation thoughts at the bottom of comment 0.

Comment 3

[Nathan LaPré]

Yup, on it! Thanks.

Comment 4

[Nathan LaPré]

Attachment: Bug 1806356: Create an accessible if the element's frame has a transform, r?Jamie

This revision changes the logic of MustCreateAccessible such that we always create
an accessible if the content's frame has been transformed. This ensures that we
have accessibles to which we apply transforms when calculating accessible bounds.
This revision also adds a test to verify that the accessible is created, even
when the element has role="presentation".

Comment 5

[Nathan LaPré]

The above patch fixes the test case in comment 0, and the minimized test case reported in Bug 1812169, so I think they're rightfully marked as duplicates. Unfortunately, the original issue with the on-hover Like option buttons (see here) still exists on LinkedIn. Seems like maybe there is something else there that I have yet to determine. Regardless, this patch is still an improvement.

Comment 6

[James Teh [:Jamie]]

While unlikely, I wonder if they're creating the div and then adding the transform later in such a way that it doesn't cause reflow. In that case, a11y wouldn't get notified about the possible change and would thus never reassess whether to create an Accessible for it.

Comment 7

[James Teh [:Jamie]]

Test case:

data:text/html,<p>1</p><div style="position: absolute; width: 600px; height: 300px;"><p id="p2">2</p></div><button onclick="document.querySelector('div').style.transform = 'translate(100%25, 100%25)';">Change</button>

Even with this patch, when you click the Change button, the div doesn't get an Accessible and thus the bounds for p2 are wrong.

It should be possible to fix this by checking for a transform (aContent->GetPrimaryFrame()->IsTransformed()) when the computed style changes and no Accessible currently exists. In that case, we should be able to call this to try inserting the Accessible (which will of course respect the usual creation rules):
document->ContentInserted(content, content->GetNextSibling());

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b27d83c0aa57
Create an accessible if the element's frame has a transform, r=Jamie

Comment 9

[Sandor Molnar[:smolnar]]

Backed out for causing for causing dt failures in devtools/client/netmonitor/test/browser_net_basic-search.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/5ed0b0b025f210958c20616305d1cb0b2d6b3fff

Push with failures

Failure log

Comment 10

[Nathan LaPré]

(In reply to James Teh [:Jamie] from comment #7)

It should be possible to fix this by checking for a transform when the computed style changes and no Accessible currently exists. In that case, we should be able to call this to try inserting the Accessible (which will of course respect the usual creation rules):

Tried this out and it seems to work, somewhat. The bounds are not immediately correct (according to NVDA), but do correct themselves if I poke the updated div with the Dev Tools (the section accessible doesn't show up until I do that, and I'm not sure why). That's weird to me. I'm concerned that the cache doesn't get updated properly. Investigating this more.

I'm going to combine these patches, I think, and try to figure out what's going on with the dev tools tests that caused failures.

Comment 11

[James Teh [:Jamie]]

I neglected to mention you should probably try this with my re-parenting patch from bug 1811972 applied. Otherwise, creating the new div acc won't cause a bounds update on the children.

Comment 12

[James Teh [:Jamie]]

Hmm, that doesn't explain why the section accessible isn't created though. That suggests the transform change isn't being picked up elsewhere and we need a reflow to pick that up. I'm not convinced the new computed style change code is working in that case.

Comment 13

[James Teh [:Jamie]]

(In reply to Nathan LaPré from comment #10)

Tried this out and it seems to work, somewhat. The bounds are not immediately correct (according to NVDA), but do correct themselves if I poke the updated div with the Dev Tools (the section accessible doesn't show up until I do that

As discussed on Zoom/Matrix, you'll need to use something similar to this rather than just frame->IsTransformed(). The reason is that the frame hasn't been updated yet when we get the style change notification. Sorry for misleading you. 😳

(In reply to Sandor Molnar from comment #9)

Backed out for causing for causing dt failures in devtools/client/netmonitor/test/browser_net_basic-search.js

I looked into this. I think what's happening here is that a node (span.treeIcon) previously didn't get an Accessible, but now it does (Presumably because it has a transform). We explicitly tell the UI a11y checks framework that it doesn't require an Accessible. However, if it does have an Accessible (despite disabling that rule), we still run the a11y checks. (Notice we return early if there's no acc, but we don't return early if there is an acc.) That rule can be disabled for longer than a single test, so we can't assume there won't be an acc just because that rule is disabled.

The fix is probably to add aria-hidden="true" (probably in a separate patch) to that span.treeIcon. We already use role="presentation", but this force creation for transforms necessarily overrides role="presentation" (because otherwise we could prune a container with a transform and role="presentation", breaking the bounds for descendants). So, I'd remove role: "presentation" and replace it with "aria-hidden": "true".

Comment 14

[James Teh [:Jamie]]

Another possibility could be that we only force creation for a transform in MustBeAccessible if the DOM node has children (aContent->HasChildren()). That won't cover some cases, but it should cover this one. It also might cause problems if anyone ever creates an empty transform and adds content to it later without reflowing, though that's ... probably unlikely?

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b8e5aeee003
Create an accessible if the element's frame has a transform, r=Jamie

Comment 16

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/5b8e5aeee003