Closed Bug 1620508 Opened 3 months ago Closed 3 months ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox-esr68 --- unaffected
firefox73 --- unaffected
firefox74 --- unaffected
firefox75 --- fixed
firefox76 --- fixed

People

(Reporter: Jamie, Assigned: eeejay)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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.

OK, that's bad.

I think that patch might need to be reverted.

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.

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.

Assignee: nobody → eitan
Status: NEW → ASSIGNED
Pushed by eisaacson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7092b787ffba
Revert change where opacity:0 makes subtrees invisible. r=Jamie

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

Flags: needinfo?(eitan)
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76

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:
Flags: needinfo?(eitan)
Attachment #9132372 - Flags: approval-mozilla-beta?

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

regression fix, approved for 75.0b3

Attachment #9132372 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.