1620508 - After bug 925209, sender headings in Facebook Messenger are exposed as invisible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[James Teh [:Jamie]]

In conversations on Facebook Messenger (messenger.com), the sender of each message is indicated with an h5. In very simplified code, it goes something like this:

<h2>Joe Blogs</h2>
...
<h3>Conversation Information</h3>
...
<h3>Messages</h3>
...
<h4>2:04 PM</h4>
<h5>Joe</h5>
...
Hi!

Those h5s are not visually shown; I guess they use thumbnails or some other way of indicating the sender. Instead of being off-screen, they use opacity: 0. That means we now expose them as invisible.

NVDA ignores the invisible state, so thankfully, they still get rendered in NVDA browse mode. I'm not sure about other AT. If we're filtering invisibles out for VoiceOver, this is going to cause a problem there.

Even when the invisible state is ignored, this causes a problem. We return early in GroupPosition if the invisible state is present. That means NVDA can't get the heading level, so it just gets reported as "heading" instead of "heading level 5" and you can't quick nav to heading level 5.

Comment 1

[Eitan Isaacson [:eeejay]]

OK, that's bad.

I think that patch might need to be reverted.

Comment 2

[Eitan Isaacson [:eeejay]]

Since we get the opacity:0 awesomeness from layout. I think it may be worth using it to mark ~OPAQUE on subtrees of opacity:0.

We ended up not needing this in mac, so I can also see wholly reverting it.

Comment 3

[James Teh [:Jamie]]

It looks like we only expose the opaque state for opacity: 1, but I'm confused about how this gets inherited through the tree:
https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/accessible/generic/Accessible.cpp#1238
Anyway, it makes sense to use this info from layout somehow, so I'm happy with either resolution here.

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1620508 - Revert change where opacity:0 makes subtrees invisible. r?Jamie!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7092b787ffba
Revert change where opacity:0 makes subtrees invisible. r=Jamie

Comment 6

[Marco Zehe (:MarcoZ)]

Eitan, please request beta uplift for this one, since the original patch landed in 75 and regressed things there. Thanks!

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/7092b787ffba

Comment 8

[Eitan Isaacson [:eeejay]]

Comment on attachment 9132372 [details]
Bug 1620508 - Revert change where opacity:0 makes subtrees invisible. r?Jamie!

Beta/Release Uplift Approval Request

• User impact if declined: Some elements that are meant to be visible to screen readers become unreadable because their context is stripped
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): I reverted a recently landed change so we are back to the very stable status quo
• String changes made/needed:

Comment 9

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9132372 [details]
Bug 1620508 - Revert change where opacity:0 makes subtrees invisible. r?Jamie!

regression fix, approved for 75.0b3

Comment 10

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/746f0b583da8