Closed Bug 758884 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: Disability Access APIs, defect)

All
Android
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: MarcoZ, Assigned: eeejay)

References

()

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

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
Version: 1.0 Branch → Trunk
Is this with the virtual d-pad or a hardware keyboard?
This is with a hardware controller.
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.
The "Talkback almost unusable" part didn't occur again, I guess this was a glitch that was remedeed by a proper reboot.
Summary: [AccessFu] Regression: virtual cursor gets stuck, renders Talkback almost unusable → [AccessFu] Regression: virtual cursor gets stuck on banner graphic of Marco's blog
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: nobody → eitan
Status: NEW → ASSIGNED
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.
First present the cursor's position, and only then modify the focus in the document.
Attachment #628187 - Flags: review?(dbolter)
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)
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 :)
(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
(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?
(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.
(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...
(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.
(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.
Just added a comment about a future optimization via IsInDocument.
Attachment #628453 - Flags: review?(dbolter)
Attachment #628453 - Flags: review?(dbolter) → review+
Attachment #628188 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Depends on: 760972
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: