Closed Bug 677154 Opened 13 years ago Closed 12 years ago

Detached document accessibility tree

Categories

(Core :: Disability Access APIs, defect, P1)

Product:
Core
Component:
Disability Access APIs
Platform:
x86_64
Windows 7
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla13
Tracking Flags:
Tracking Status
firefox10 --- wontfix
firefox11 --- wontfix
firefox12 --- fixed
firefox-esr10 --- wontfix

People

(Reporter: Jamie, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

patch2
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.
Blocks: treea11y
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.
My bad. This doesn't appear to work on newly created accounts. :(
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.
I can definitely reproduce this in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110922 Firefox/9.0a1.
(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.
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.
(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.
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.
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.
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
Jamie, maybe you could share your profile with us (sending it by email excluding your private data like passwords)?
I've shared a profile privately with which I can reproduce the issue.
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
(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?
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.
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
Attached patch patch (obsolete) — — Splinter Review 
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)
> 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 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.
Attached patch patch2 — — Splinter Review 
added scrollbar styles check
added mochitests
Attachment #597680 - Attachment is obsolete: true
Attachment #598222 - Flags: review?(marco.zehe)
Comment on attachment 598222 [details] [diff] [review]
patch2

r=me for the tests.
Attachment #598222 - Flags: review?(marco.zehe) → review+
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
Jamie, could you please check other examples if they were fixed too?
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
Alex, is this a regression from the big frames rework of Firefox 4 days? Can we justify getting this approved for Aurora and Beta?
(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.
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 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+
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: