1332731 - TalkBack stopped working in web content

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Roy Nickelson]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161208153507

Steps to reproduce:

start TalkBack
Navigate to 
https://labs.ssbbartgroup.com/index.php/ARIA_Checkbox_role
Navigate to one of the check boxes



Actual results:

Talkback announces Button


Expected results:

TalkBack should announce check box

Comment 1

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi Sorina,
If this bug can be reproduced, could you provide a regression window?

Comment 2

[Sorina Florean [:sflorean]]

Regression window:
Last known good build: 2016-12-10
First known bad build: 2016-12-11

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8404d26166a35406f46ff237ed132735c98882b2&tochange=c51e7406d7b2e2246a1ece0d8989282ca752039f

Device: Huawei MediaPad M2 (Android 5.1.1).

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

Changes from Jim & Eugen during that day.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Follow-up to fix accessibility breakage (v1)

Follow-up to fix breakage in accessibility caused by the bundle
conversion. In particular, optString(foo) should have been converted to
getString(foo, "") because optString returns "" by default.

Also fix a small bug in Presentation.jsm where an array or null should
be used instead of a string.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Follow-up to fix accessibility breakage (v2)

Updated patch

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffddbc50a846
Follow-up to fix accessibility breakage; r=sebastian

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ffddbc50a846

Comment 9

[Sorina Florean [:sflorean]]

Verified as fixed in build 54.0a1 (2017-02-16);
Device: Asus ZenPad 8 (Android 6.0.1).

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8834066 [details] [diff] [review]
Follow-up to fix accessibility breakage (v2)

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1321418
[User impact if declined]: TalkBack accessibility feature does not work on web content
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes, and Aurora/Beta.
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Patch has been on 54+ for a while, but was never uplifted to 53.
[String changes made/needed]: None

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Liz, we should consider including this fix in 53.0.1. This fix has been on 54 for almost 2 months and verified by SoftVision.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

I've seen several recent examples of a regression marked only as tracking-fennec:+ but not marked as affected for a particular version. I think we have a good chance here to improve how our triage systems work together. 

I'd like to suggest: when you see new issues, please add the "regression" keyword and possibly "regressionwindow-wanted".  We should be all be proactive about it. When did the issue start? How bad is this regression for users? What versions does it affect? Should we uplift the fix?   

We need to ask those questions so that we can avoid shipping regressions, or fix those regressions as soon as possible. So, we have to just go a little bit beyond landing the patch and verifying the bug was fixed in Nightly. 

This will really help, because then you will get all the "machinery" of QE, release management, and platform triage (engineering managers) noticing fennec issues, and getting the fixes where they should be.  

n-i to some folks in this bug, to make sure you see my comment, not because this needs any reply. 
I will also send my suggestion to the release-drivers mailing list.

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8834066 [details] [diff] [review]
Follow-up to fix accessibility breakage (v2)

Broke accessibility on release, let's uplift this and test.

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12)
> I'd like to suggest: when you see new issues, please add the "regression"
> keyword and possibly "regressionwindow-wanted".  We should be all be
> proactive about it. When did the issue start? How bad is this regression for
> users? What versions does it affect? Should we uplift the fix?

Got it and I agree. I should have done better here, and I will remember these points in the future. Thanks!

Comment 15

[Sorina Florean [:sflorean]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12)
> I've seen several recent examples of a regression marked only as
> tracking-fennec:+ but not marked as affected for a particular version. I
> think we have a good chance here to improve how our triage systems work
> together. 
> 
> I'd like to suggest: when you see new issues, please add the "regression"
> keyword and possibly "regressionwindow-wanted".  We should be all be
> proactive about it. When did the issue start? How bad is this regression for
> users? What versions does it affect? Should we uplift the fix?   
> 
> We need to ask those questions so that we can avoid shipping regressions, or
> fix those regressions as soon as possible. So, we have to just go a little
> bit beyond landing the patch and verifying the bug was fixed in Nightly. 
> 
> This will really help, because then you will get all the "machinery" of QE,
> release management, and platform triage (engineering managers) noticing
> fennec issues, and getting the fixes where they should be.  
> 
> n-i to some folks in this bug, to make sure you see my comment, not because
> this needs any reply. 
> I will also send my suggestion to the release-drivers mailing list.

Will do that in the feature. Thanks!

Comment 16

[Max Liu [:maliu]]

Agree. We will also do that in future Fennec triage meeting. Thanks, Sebastian. :)

Comment 17

[Bogdan Surd, QA [:bsurd, NI]]

Hello, 

I've verified this issue in 54.0b2 Beta build and the latest Nightly, the issue seems to be reproducible, as described in Comment 0. Could this be a new regression maybe or was the fix never applied to these builds?

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Hi Bogdan, It looks like the fix hasn't landed on release yet, but I'm a bit worried that it also may not be on 54.  Wes can you take a look and make sure this has really landed on 54?

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #18)
> Hi Bogdan, It looks like the fix hasn't landed on release yet, but I'm a bit
> worried that it also may not be on 54.  Wes can you take a look and make
> sure this has really landed on 54?

Patch landed on m-c when m-c was 54. I'd assume a new regression. Can qa track down a new regression range to see when it popped back up on 54? Perhaps :bogdan can run a Nightly build from right after this originally landed to see if the fix could be verified?

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

Bogdan, can you follow up?  
Jim, can your team in Taipei help? 
This is our main blocker right now for the fennec 53 dot release.  
The patch here hasn't landed on mozilla-release yet. But, we think it may not have worked in 54. 
It sounds like there is potentially an issue on 53, 54, and 55. Maybe a different cause than the original issue?

Comment 21

[(inactive) Jim Chen [:jchen] [:darchons]]

The STR in comment 0 works for me. The checkboxes are announced as "check button", which is the expected behavior AFAICT.

Comment 22

[Bogdan Surd, QA [:bsurd, NI]]

(In reply to Roy Nickelson from comment #0)
> User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101
>
> Expected results:
> 
> TalkBack should announce check box

 The expected result is that they should be announced as check boxes, not buttons. If it's ok that they are announced as buttons, the issue can be considered as fixed and verified.

Comment 23

[Roy Nickelson]

If an element has a Roll of Checkbox then it should be announced as a checkbox. A checkbox is not the same as a button. The roll indicates to the user how to interact with the control.  
The expected behavior is Talkback will announce something like option 1 check box not checked when activating the check box by Double tapping on it Talkback should announce checked.

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

I still can't tell, on which branches does this now work, and on which is it still broken? 
Does TalkBack work in general for content now? And on which branches? 

I would think that check boxes are different than buttons, since they have different behavior for users. 

David, I wonder if you can help here since this is a pretty big accessibility issue affecting our release population.

Comment 25

[James Willcox (:snorp) ([e-mail]) (he/him)]

It looks like this is fixed everywhere except 53, but it looks like it will be in 53.0.1.

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Wes could you land this on m-r? Thanks!

Comment 27

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Roy Nickelson from comment #23)
> If an element has a Roll of Checkbox then it should be announced as a
> checkbox. A checkbox is not the same as a button. The roll indicates to the
> user how to interact with the control.  
> The expected behavior is Talkback will announce something like option 1
> check box not checked when activating the check box by Double tapping on it
> Talkback should announce checked.

We should probably open another bug about changing "check button" to "check box". Unfortunately there was a different bug that broken TalkBack completely, and that issue ended up being worked on in this bug.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/5cbf464688a4

Comment 30

[Bogdan Surd, QA [:bsurd, NI]]

Device: Honor 8 (Android 6.0)
Builds:
 - 53.0.1 (Build 2)
 - 54.0b2