Closed
Bug 677154
Opened 13 years ago
Closed 12 years ago
Detached document accessibility tree
Categories
(Core :: Disability Access APIs, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla13
People
(Reporter: Jamie, Assigned: surkov)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
6.36 KB,
patch
|
MarcoZ
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Set up steps (only need to be done once): 1. Go to http://www.pennytel.com/. 2. Sign up for a "Personal VoIP-Free Access" account. You should only need to complete the first screen. You can just close the window requesting address details, etc. 3. Visit https://www.pennytel.com/login.jsp and log in. 4. Disable the soft phone to make life easier and the test simpler. 5. Log out. Str: 1. With NVDA running, visit https://www.pennytel.com/login.jsp and log in. 2. Select "Profile" from the main navigation bar. 3. Retrieve and examine the tree of the accessible for the document in the last iframe (where the main content of the page is presented). Expected: There should be quite a lot of accessibles, starting with a form just below the document. Actual: There is just a "text frame" accessible. The tag attribute reveals this to be for the "body" tag. Note: If this doesn't occur, repeat from step 2. It doesn't appear to be 100% reproduceable, but it does happen more often than not for me. 4. Press tab a few times to focus inside this document tree. 5. Retrieve the accessible for the focus. 6. Walk up its parents using IAccessibleObject::accParent to find the document accessible. Expected: You should eventually reach the document accessible. Actual: When you call IAccessible::accParent on the form accessible, you get E_UNEXPECTED.
Assignee | ||
Comment 1•13 years ago
|
||
I did steps 1-3 (str). The tree I see: document "Welcome to PenyTel :: User Interface" table row cell section section (has children) section internal frame document "Welcome to PenyTel :: User Interface" text container (HTML body) form (HTML form) table (has children) Am I looking at correct document? I'm running NVDA 2011.2b3.
Reporter | ||
Comment 2•13 years ago
|
||
My bad. This doesn't appear to work on newly created accounts. :(
Reporter | ||
Comment 3•13 years ago
|
||
This document seems to cause the same problem, but it seems to be 100% reproduceable for me and is much simpler to reproduce as no login is needed: http://www.eveready.com/batteries/Pages/eveready-gold.aspx Tested with Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110917 Firefox/9.0a1 Will test with updated build later, but wanted to note the URL before I lose it.
Comment 4•13 years ago
|
||
I can definitely reproduce this in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110922 Firefox/9.0a1.
Assignee | ||
Comment 5•13 years ago
|
||
(In reply to James Teh [:Jamie] from comment #3) > This document seems to cause the same problem, but it seems to be 100% > reproduceable for me and is much simpler to reproduce as no login is needed: > http://www.eveready.com/batteries/Pages/eveready-gold.aspx > Tested with Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110917 > Firefox/9.0a1 > Will test with updated build later, but wanted to note the URL before I lose > it. Can you give me steps to reproduce? I don't see any iframe there and missed content.
Comment 6•13 years ago
|
||
Just open the page Jamie points to. I only see two skip to... links in the NVDA virtual buffer. The rest of the page is NOT there. But if I tab through the page, NVDA reports focus on elements that i don't find in the accessibility tree.
Reporter | ||
Comment 7•13 years ago
|
||
(In reply to alexander surkov from comment #5) > > http://www.eveready.com/batteries/Pages/eveready-gold.aspx > Can you give me steps to reproduce? I don't see any iframe there and missed > content. Sorry for not providing more thorough info. I was in a hurry, but wanted to note the URL. There's no iframe, but there is definitely a great deal of missed content. Str: 1. With NVDA running, visit http://www.eveready.com/batteries/Pages/eveready-gold.aspx 2. Tab to the "HOME" link. 3. Retrieve the accessible for the focus. 4. Walk up its parents using IAccessibleObject::accParent to find the document accessible. Expected: You should eventually reach the document accessible. Actual: When you call IAccessible::accParent on the accessible for the div with @id="s4-bodyContainer", you get E_UNEXPECTED.
Assignee | ||
Comment 8•13 years ago
|
||
Hm, can't reproduce it. get_accParent can return E_UNEXPECTED iff when there's no Gecko parent. So I listed gecko's parent chain when Gecko focus event is handled and it looks ok when for "HOME" link.
Reporter | ||
Comment 9•13 years ago
|
||
This is really odd. I can't reproduce this with a clean profile. However, with my normal profile, it still happens even in safe mode. Marco, can you reproduce this with a clean profile? This suggests that some pref or setting must be affecting this, but I have no idea what.
Reporter | ||
Comment 10•12 years ago
|
||
Uh oh. Yet another case of this reported by an NVDA user: http://www.wssdemo.com/default.aspx On this page, nothing past the toolbar is attached to the accessibility tree. Again, frustratingly, if I use a clean profile, I can't reproduce it, but with my normal profile (even with safe mode), it happens every time. I don't know what's causing this, but it is affecting multiple users. Str: 1. With NVDA running, visit the URL above. 2. Move focus to the top of the document by pressing control+k then tab. 3. Press NVDA+space to force focus mode so that NVDA doesn't intercept the tab key. 4. Press tab 11 times. 5. Retrieve the accessible for the focus. 6. Walk up its parents using IAccessible::accParent to find the document accessible. Expected: You should eventually reach the document accessible. Actual: When you call IAccessible::accParent on the accessible for the div with @id="s4-bodyContainer", you get E_UNEXPECTED. Related NVDA ticket: http://www.nvda-project.org/ticket/2059
Assignee | ||
Comment 11•12 years ago
|
||
Jamie, maybe you could share your profile with us (sending it by email excluding your private data like passwords)?
Reporter | ||
Comment 12•12 years ago
|
||
I've shared a profile privately with which I can reproduce the issue.
Comment 13•12 years ago
|
||
This bug is now being exposed in the latest layout of Gmail if standard settings are active and for example letters j and k work to move from message to message. Bumping priority and severity accordingly.
Severity: normal → major
Priority: -- → P1
Assignee | ||
Comment 14•12 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #6) > Just open the page Jamie points to. I only see two skip to... links in the > NVDA virtual buffer. The rest of the page is NOT there. But if I tab through > the page, NVDA reports focus on elements that i don't find in the > accessibility tree. I can reproduce this part when Firefox window is maximized (thanks to Jamie's profile), otherwise the page works fine. But I can't reproduce Jamie's comment #7: (In reply to James Teh [:Jamie] from comment #7) > Str: > 1. With NVDA running, visit > http://www.eveready.com/batteries/Pages/eveready-gold.aspx > 2. Tab to the "HOME" link. > 3. Retrieve the accessible for the focus. > 4. Walk up its parents using IAccessibleObject::accParent to find the > document accessible. > Expected: You should eventually reach the document accessible. > Actual: When you call IAccessible::accParent on the accessible for the div > with @id="s4-bodyContainer", you get E_UNEXPECTED. I tried by DOMi to listen focus event and print parents chain (crossplatform part works). I tried to inspect IAccessible::accParent on div@id="s4-bodyContainer" by accprobe and it returns a parent. Is the magic that I should get IAccessible::accParent when I handle MSAA focus event?
Assignee | ||
Comment 15•12 years ago
|
||
Hm, you know, I can see incomplete tree (div@id="s4-bodyContainer" part is missed), maybe I tried accParent after the tree was magically updated (but not NVDA vb). I'll look into this more.
Assignee | ||
Comment 16•12 years ago
|
||
div@id="s4-workspace" that contains div@id="s4-bodyContainer" is not accessible initially. So initially HTML form accessible contains div@id="s4-bodyContainer" accessible as a child. Eventually div@id="s4-workspace" gets an accessible eventually and the accessible is created when HTML form accessible children are updated. Since that update is not for div@id="s4-workspace" then we don't cache children for it, therefore div@id="s4-bodyContainer" subtree gets unattached. div@id="s4-workspace" has scroll frame and its accessibility status depends whether the scrollframe is focusable or not. The problem is focusable state can be changed during frame lifecycle. This part of code (http://hg.mozilla.org/mozilla-central/diff/bd1754632d8c/layout/generic/nsFrame.cpp) defines focusable state for scrollframe. When initial accessible tree is created then scrollFrame->GetScrollRange() is empty what makes it inaccessible. After sometime scrollrange gets unempty so we create an accessible as described above. I tempted to say this is regression from bug 679791 (targeted to Mozilla 9) which changed focuability logic for scroll frames but bug 679791 was fixed 17 august but this one was reported 07 Aug. Anyway cc'ing Roc and Josh for help. The fix could be: notify accessibility when result of nsIFrame::IsFocusable is changed during frame lifecycle.
Keywords: regression
Assignee | ||
Comment 17•12 years ago
|
||
Since result !scrollFrame->GetScrollRange().IsEqualEdges(nsRect(0, 0, 0, 0)) of nsIFrame::IsFocusable (http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp#7206) can be changed during frame life cycle without any notification to a11y then IsFocusable() check doesn't work for us (http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#972). So at minimum we can create an accessible for nsHTMLScrollFrame if it's not root of native anonymous subtree (port from nsIFrame::IsFocusable), at least I didn't noticed accessible tree changes in general. If the result of scrollFrame->GetScrollbarStyles() != nsIScrollableFrame::ScrollbarStyles(NS_STYLE_OVERFLOW_HIDDEN, NS_STYLE_OVERFLOW_HIDDEN) cannot be changed during frame life cycle then it's worth to add this check too. If it can be changed then alternatively we could listen style changes and notify a11y. But in general I'm not sure what nsHTMLScrollFrame is used for, i.e. which HTML elements have this frame and under which circumstances. Knowing this could help me to answer whether we can keep it accessible always or update accessible tree when for example overflow style is changed. Also, I failed to create a testcase when result of scrollFrame->GetScrollRange().IsEqualEdges() is changed during frame life cycle. Boris, that'd be great if you can help me to find answers.
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #597680 -
Flags: review?(bzbarsky)
Comment 18•12 years ago
|
||
> But in general I'm not sure what nsHTMLScrollFrame is used for Scrolling. Basically any time you have a block with overflow:scroll or overflow:auto. Also used for the viewport scrollbars. I _think_ that the return value of GetScrollbarStyles() can't change without reframing, but check with roc? > Also, I failed to create a testcase when result of > scrollFrame->GetScrollRange().IsEqualEdges() is changed during frame life cycle. Any testcase that dynamically changes the height of the content inside an overflow:scroll block should to it.
Comment 19•12 years ago
|
||
Comment on attachment 597680 [details] [diff] [review] patch "regardless of"
Attachment #597680 -
Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky (:bz) from comment #18) > I _think_ that the return value of GetScrollbarStyles() can't change without > reframing, but check with roc? I think that's right.
Assignee | ||
Comment 21•12 years ago
|
||
added scrollbar styles check added mochitests
Attachment #597680 -
Attachment is obsolete: true
Attachment #598222 -
Flags: review?(marco.zehe)
Comment 22•12 years ago
|
||
Comment on attachment 598222 [details] [diff] [review] patch2 r=me for the tests.
Attachment #598222 -
Flags: review?(marco.zehe) → review+
Assignee | ||
Comment 23•12 years ago
|
||
inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/4c23b6fefdb2
Assignee | ||
Comment 24•12 years ago
|
||
forgot to fix misspelling (comment #19) https://hg.mozilla.org/integration/mozilla-inbound/rev/c4488d89ae97
Comment 25•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4c23b6fefdb2 https://hg.mozilla.org/mozilla-central/rev/c4488d89ae97
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Assignee | ||
Comment 26•12 years ago
|
||
Jamie, could you please check other examples if they were fixed too?
Comment 27•12 years ago
|
||
I can verify that gmail is fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/13.0a1 Firefox/13.0a1
Comment 28•12 years ago
|
||
Alex, is this a regression from the big frames rework of Firefox 4 days? Can we justify getting this approved for Aurora and Beta?
Assignee | ||
Comment 29•12 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #28) > Alex, is this a regression from the big frames rework of Firefox 4 days? I think changes in layout can tweak bug reproducibility. But yes, Firefox 4 accessible tree rework is a root cause. > Can > we justify getting this approved for Aurora and Beta? we should do this. from technical side: this patch shouldn't deliver any regressions because all it can do is to alter an accessible tree the way screen readers don't care. In 99% no change in tree, without the patch certain web sites accessibility is dead that's a reason why it's important to port it.
Assignee | ||
Comment 30•12 years ago
|
||
Comment on attachment 598222 [details] [diff] [review] patch2 [Approval Request Comment] Regression caused by (bug #): not known User impact if declined: certain web cites are not accessible Testing completed (on m-c, etc.): yes Risk to taking this patch (and alternatives if risky): low String changes made by this patch: no
Attachment #598222 -
Flags: approval-mozilla-aurora?
Comment 31•12 years ago
|
||
Comment on attachment 598222 [details] [diff] [review] patch2 [Triage Comment] This is low risk and in support of accessibility - approved for Aurora 12.
Attachment #598222 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 32•12 years ago
|
||
Landed on Aurora at Surkov's request: http://hg.mozilla.org/releases/mozilla-aurora/rev/c2749c1ced57
status-firefox-esr10:
--- → affected
status-firefox10:
--- → affected
status-firefox11:
--- → affected
status-firefox12:
--- → fixed
Flags: in-testsuite+
Reporter | ||
Comment 33•12 years ago
|
||
All other test cases verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120226 Firefox/13.0a1
Status: RESOLVED → VERIFIED
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•