[AccessFu] Regression: virtual cursor gets stuck on banner graphic of Marco's blog

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: MarcoZ, Assigned: eeejay)

Tracking

({regression})

Trunk
mozilla15
All
Android
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
STR:
1. With the May 26, 2012 build of Android nightly, Talkback enabled, go to http://www.marcozehe.de
2. Start navigating with left and right d-pad or controller.

Result: Once v cursor lands on the graphic following the blog tag line, third or fourth element in all, v cursor gets stuck and Talkback becomes very unresponsive, even after closing nightly.

This is a very recent regression.
Component: Build Config → Disability Access APIs
QA Contact: build-config → accessibility-apis
(Reporter)

Updated

5 years ago
Version: 1.0 Branch → Trunk
(Assignee)

Comment 1

5 years ago
Is this with the virtual d-pad or a hardware keyboard?
(Reporter)

Comment 2

5 years ago
This is with a hardware controller.
(Reporter)

Comment 3

5 years ago
Just reconfirmed this in the latest nightly build (May 29). On www.marcozehe.de, the virtual cursor gets stuck on the graphic element following the "Musings ..." text, which is the third item on the page the pivot navigates to.

The *only* thing that works is navigating upwards to the native UI and loading a different page. Getting stuck on that graphic is 100% reproducible for me.
(Reporter)

Comment 4

5 years ago
The "Talkback almost unusable" part didn't occur again, I guess this was a glitch that was remedeed by a proper reboot.
(Reporter)

Updated

5 years ago
Summary: [AccessFu] Regression: virtual cursor gets stuck, renders Talkback almost unusable → [AccessFu] Regression: virtual cursor gets stuck on banner graphic of Marco's blog
(Assignee)

Comment 5

5 years ago
I see this error in adb logcat:

05-29 09:41:32.751  2507  2517 I Gecko   : [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIAccessible.parent]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/accessibility/Presenters.jsm :: <TOP_LEVEL> :: line 367"  data: no]

Coming up with a fix soon.
(Assignee)

Updated

5 years ago
Assignee: nobody → eitan
Status: NEW → ASSIGNED
(Assignee)

Comment 6

5 years ago
The error is due to the tree mutations that happen when the CSS :focus selector is triggered when we focus on the link under the cursor. In other words, once we focus the pivot position it becomes invalid. Schrodinger's cat indeed! This is the cost of modifying the model from the view in an MVC design. I still think it is worth changing the focus with the virtual cursor.
(Assignee)

Comment 7

5 years ago
Created attachment 628187 [details] [diff] [review]
(part 1/2) Present virtual cursor before modifying focus.

First present the cursor's position, and only then modify the focus in the document.
Attachment #628187 - Flags: review?(dbolter)
(Assignee)

Comment 8

5 years ago
Created attachment 628188 [details] [diff] [review]
(part 2/2) Check if position is still a descendant of pivot's root before moving in relation to it.

IsInDocument seems to be the wrong test. It is hard to reproduce, but sometimes the accessibles are in the doc's accessible map, but are detached from the heirarchy. I don't understand the accessible lifecycle well enough, but checking if the position is a descendant of the pivot root seems to be more correct anyway.
Attachment #628188 - Flags: review?(dbolter)
(Reporter)

Comment 9

5 years ago
Oh, it's the skip link! It is being brought into the visible view once it gets focus. Previously, it is off-screen.
Attachment #628187 - Flags: review?(dbolter) → review+
Attachment #628188 - Flags: review?(dbolter) → review+
Requesting Alexander's feedback on comment 8 :)

Comment 11

5 years ago
(In reply to Eitan Isaacson [:eeejay] from comment #8)
> Created attachment 628188 [details] [diff] [review]
> (part 2/2) Check if position is still a descendant of pivot's root before
> moving in relation to it.
> 
> IsInDocument seems to be the wrong test. It is hard to reproduce, but
> sometimes the accessibles are in the doc's accessible map, but are detached
> from the heirarchy. I don't understand the accessible lifecycle well enough,
> but checking if the position is a descendant of the pivot root seems to be
> more correct anyway.

if you workaround IsInDocument issue and don't have reliable steps to reproduce then add an assertion
(Assignee)

Comment 12

5 years ago
(In reply to alexander :surkov from comment #11)
> (In reply to Eitan Isaacson [:eeejay] from comment #8)
> > Created attachment 628188 [details] [diff] [review]
> > (part 2/2) Check if position is still a descendant of pivot's root before
> > moving in relation to it.
> > 
> > IsInDocument seems to be the wrong test. It is hard to reproduce, but
> > sometimes the accessibles are in the doc's accessible map, but are detached
> > from the heirarchy. I don't understand the accessible lifecycle well enough,
> > but checking if the position is a descendant of the pivot root seems to be
> > more correct anyway.
> 
> if you workaround IsInDocument issue and don't have reliable steps to
> reproduce then add an assertion

A parallel discussion is happening in bug 759618. I think I will modify IsRootDescendant here, to traverse up the tree and if it is a document do IsInDocument.

How do you suggest asserting? First traverse up the tree and then compare with IsInDocument? Keep the assertion there until we find a solution and could drop up-tree traversal in document roots?

Comment 13

5 years ago
(In reply to Eitan Isaacson [:eeejay] from comment #12)

> A parallel discussion is happening in bug 759618. 

there IsRootDescendant makes sense (because you pass arbitrary anchor), here it's used to workaround bug.

> I think I will modify
> IsRootDescendant here, to traverse up the tree and if it is a document do
> IsInDocument.

not sure how you're going to connect IsInDocument and IsRootDescendant, anyway, I guess I'll see a patch.

> How do you suggest asserting? First traverse up the tree and then compare
> with IsInDocument? 

yes, compare results of IsInDocument and IsRootDescendant, if different then assert

>Keep the assertion there until we find a solution and
> could drop up-tree traversal in document roots?

keep assertion to find a bug, when you see an assertion then you can attach a debugger and see what happens. If we fix it then let's get back IsInDocument() check or whatever appropriate.
(Assignee)

Comment 14

5 years ago
(In reply to alexander :surkov from comment #13)
> (In reply to Eitan Isaacson [:eeejay] from comment #12)
> >Keep the assertion there until we find a solution and
> > could drop up-tree traversal in document roots?
> 
> keep assertion to find a bug, when you see an assertion then you can attach
> a debugger and see what happens. If we fix it then let's get back
> IsInDocument() check or whatever appropriate.

So you are suggesting I do this locally, not propose a patch with an assertion...

Comment 15

5 years ago
(In reply to Eitan Isaacson [:eeejay] from comment #14)

> So you are suggesting I do this locally, not propose a patch with an
> assertion...

no, I suggest a patch with assertion. Anybody can debug it.
(Assignee)

Comment 16

5 years ago
(In reply to alexander :surkov from comment #15)
> (In reply to Eitan Isaacson [:eeejay] from comment #14)
> 
> > So you are suggesting I do this locally, not propose a patch with an
> > assertion...
> 
> no, I suggest a patch with assertion. Anybody can debug it.

I managed to reliably reproduce, and I filed bug 759875. So I am just putting in a to-do comment to use a more optimized path once that bug is fixed.
(Assignee)

Comment 17

5 years ago
Created attachment 628453 [details] [diff] [review]
Bug 758884 - (part 2/2) Check if position is still a descendant of pivot's root before moving in relation to it.

Just added a comment about a future optimization via IsInDocument.
Attachment #628453 - Flags: review?(dbolter)
Attachment #628453 - Flags: review?(dbolter) → review+
(Assignee)

Comment 18

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/d3092b482b0f
http://hg.mozilla.org/integration/mozilla-inbound/rev/b7728496df08
Target Milestone: --- → mozilla15
(Assignee)

Updated

5 years ago
Attachment #628188 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/d3092b482b0f
https://hg.mozilla.org/mozilla-central/rev/b7728496df08
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
Depends on: 760972
You need to log in before you can comment on or make changes to this bug.