Closed Bug 1182189 Opened 9 years ago Closed 9 years ago

Accessibility Regression in Utility tray exclusive visibility.

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::System
Platform:
All
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(b2g-v2.2 unaffected, b2g-master fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
b2g-v2.2 --- unaffected
b2g-master --- fixed

People

(Reporter: yzen, Assigned: lchang)

References

Details

(Keywords: access, regression, Whiteboard: [systemsfe])

Attachments

(1 file, 1 obsolete file)

[gaia] luke-chang:1182189_separate_hierarchy_focus > mozilla-b2g:master
46 bytes, text/x-github-pull-request
johnhu
: review+
Details | Review 
Utility tray view is no longer exclusively visible to the screen reader.

Steps to reproduce:
* With screen reader enabled open a utility tray by sliding 2 fingers down (fast) from the top edge of the screen
* Screen reader focus correctly jumps to the date on top of the screen
* Swipe left to jump to previous item

Observed: Screen reader jumps onto an item on the homescreen.
Expected: Screen reader must not jump onto the homescreen when the utility tray is open. The only things, navigable by the screen reader should be the ones in the utility tray and the status bar.

Status bar is again no longer accessible to the screen reader user. I suspect it happened after the last update to the status bar that happened a couple of weeks before. We again have the status bar shadow instead of the real thing displayed on screen so it can only be accessed via the swipe to navigate and not explore by touch.
Please ignore this part of the description: 

> Status bar is again no longer accessible to the screen reader user. I
> suspect it happened after the last update to the status bar that happened a
> couple of weeks before. We again have the status bar shadow instead of the
> real thing displayed on screen so it can only be accessed via the swipe to
> navigate and not explore by touch.
Confirmed regression from Flame 2.2. Working on getting the window.
status-b2g-v2.2: --- → unaffected
status-b2g-master: --- → affected
QA Contact: pcheng
Disregard comment 3. On affected builds just keep swiping on either right or left will eventually put focus to Homescreen.
***There are a bunch of broken builds in between last working and first broken builds so the pushlog is big. Inbound pushlog is even bigger than tinderbox central pushlog so I'm posting the central window here.

Tinderbox central regression window:

Last Working
Device: Flame
BuildID: 20150128183748
Gaia: 9d2378a9ef092ab1fc15c3a9f7fc4171aab59d57
Gecko: 6bfc0e1c4b29
Version: 38.0a1 (2.5 Master) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

First Broken
Device: Flame
BuildID: 20150129150231
Gaia: 321db3aed93a21f719fa918356f4200b8f465ac7
Gecko: 4380ed39de3a
Version: 38.0a1 (2.5 Master) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Last Working Gaia First Broken Gecko - no repro
Gaia: 9d2378a9ef092ab1fc15c3a9f7fc4171aab59d57
Gecko: 4380ed39de3a

Last Working Gecko First Broken Gaia - repro
Gaia: 321db3aed93a21f719fa918356f4200b8f465ac7
Gecko: 6bfc0e1c4b29

Gaia pushlog:
https://github.com/mozilla-b2g/gaia/compare/9d2378a9ef092ab1fc15c3a9f7fc4171aab59d57...321db3aed93a21f719fa918356f4200b8f465ac7

Possibly caused by changes made in Bug 1119704.
Blocks: 1119704
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: regressionwindow-wanted
Hermes, can you take a look at this or assign it to the proper person who can fix this? This might have been caused by the landing for bug 1119704. The author is no longer available.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(hcheng)
Hi Tim, do you know who can handle bug 1119704?
Flags: needinfo?(hcheng) → needinfo?(timdream)
John, sorry for the ni; is this something you could help?
Flags: needinfo?(timdream) → needinfo?(im)
Sure,

I can handle this.
Assignee: nobody → im
Flags: needinfo?(im)
The correct way is to set all underlying iframe and system UI as screen reader hidden while utility tray is shown. I will try to check that.

According to the patch of bug 1119704, the issue should be reproduced before that patch while we have system UI under utility tray.
The root cause is the patch of bug 1119704. So, comment 10 is wrong.

At that patch, we create setHierarchy function at UtilityTray but not well implemented. It only returns false all the time. When a hierarchy structure is changed, HierarchyManager will set the last top-most UI as inactive and set the new top-most UI as active only if setHierarchy function returns true. Sadly, we don't implement it. That's why we have this regression.
Comment on attachment 8633920 [details] [review]
[gaia] john-hu:bug-1182189-hide-underlay-while-utility-tray-shown > mozilla-b2g:master

Tim,

Please review this patch. I had implemented the setHierarchy method.

BTW, I cannot understand why they always returned false. But it should be wrong to deal with this case. If you know about this, please let me know. Thanks.
Attachment #8633920 - Flags: review?(timdream)
Comment on attachment 8633920 [details] [review]
[gaia] john-hu:bug-1182189-hide-underlay-while-utility-tray-shown > mozilla-b2g:master

Thanks!
Attachment #8633920 - Flags: review?(timdream) → review+
It seems gij6 is always failed. I will check it.
Ok, I got why they always returned false.

We have a feature that show keyboard selector at utility tray while keyboard is visible. When we implement setHierarchy correctly at utility tray, hierarchy manager will ask active app to blur itself. This breaks the keyboard feature. If we always return false, the keyboard feature works but accessibility feature breaks.

The main issue of this is we use hierarchy manager to handle both focus and hierarchy (or UI layers). It is also mentioned by alive at [1]. The correct patch should separate focus manager from hierarchy manager for handling focus. And hierarchy manager handles layers and the visible of accessibility. Focus manager may or may not change focus based on hierarchy stack of hierarchy manager. If we do so at this bug, it may take a lot of time to implement it.

Yura:

May we know the priority of this bug? If it is urgent, we may use workaround to fix it. If not, we would like to separate them.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1115508#c22
Flags: needinfo?(yzenevich)
Comment on attachment 8633920 [details] [review]
[gaia] john-hu:bug-1182189-hide-underlay-while-utility-tray-shown > mozilla-b2g:master

>https://github.com/mozilla-b2g/gaia/pull/30967
Attachment #8633920 - Attachment is obsolete: true
(In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #16)
> Ok, I got why they always returned false.
> 
> We have a feature that show keyboard selector at utility tray while keyboard
> is visible. When we implement setHierarchy correctly at utility tray,
> hierarchy manager will ask active app to blur itself. This breaks the
> keyboard feature. If we always return false, the keyboard feature works but
> accessibility feature breaks.
> 
> The main issue of this is we use hierarchy manager to handle both focus and
> hierarchy (or UI layers). It is also mentioned by alive at [1]. The correct
> patch should separate focus manager from hierarchy manager for handling
> focus. And hierarchy manager handles layers and the visible of
> accessibility. Focus manager may or may not change focus based on hierarchy
> stack of hierarchy manager. If we do so at this bug, it may take a lot of
> time to implement it.
> 
> Yura:
> 
> May we know the priority of this bug? If it is urgent, we may use workaround
> to fix it. If not, we would like to separate them.
> 
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1115508#c22

Hi John, it'd be better to have a proper fix over a workaround. Do you think it could be possible to fit the fix in 2.5? Hopefully that's not too urgent..
Flags: needinfo?(yzenevich)
(In reply to Yura Zenevich [:yzen] On PTO until July 28th from comment #18)
> Hi John, it'd be better to have a proper fix over a workaround. Do you think
> it could be possible to fit the fix in 2.5? Hopefully that's not too urgent..

I personally may not have time to do the refactoring. But we may find someone to keep it in progress.
Assignee: im → lchang
I'll try my best.
Status: NEW → ASSIGNED
Above is a WIP patch and I'll continue with the tests.
Comment on attachment 8637798 [details] [review]
[gaia] luke-chang:1182189_separate_hierarchy_focus > mozilla-b2g:master

Hi John,

I've separated "setFocus" methods from all current "setHierarchy" methods in corresponding modules. Could you please take a look? Thanks.
Attachment #8637798 - Flags: review?(im)
Comment on attachment 8637798 [details] [review]
[gaia] luke-chang:1182189_separate_hierarchy_focus > mozilla-b2g:master

Looks good to me. Thanks. Please check the tree herder result. It looks failed to start the test.
Attachment #8637798 - Flags: review?(im) → review+
Luke, did this cause bug 1191671?
Flags: needinfo?(lchang)
I think not. I checkout'ed the merging commit in comment 25 and can't reproduce bug 1191671. I'm trying to bisect it and will update my result in that bug soon.
Flags: needinfo?(lchang)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: