242594 - Nodeless text frames for ordered list item ordinals not exposed

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

Steps:
1. Open http://www.corecss.com/examples/15-02-list-style-type_css1-values.html
2. Run accessibility test tool

What happens:
The text before the period numbering each list item is not exposed

Related:
We probably don't expose text in :before and :after
Let's make sure both things work.

Comment 1

[Aaron Leventhal]

This seems to be the same problem with :before and :after

Comment 2

[Aaron Leventhal]

Attachment: Test case #2

Comment 3

[Aaron Leventhal]

Other related issues:
* can't be selected
* caret browsing through these things doesn't work, so the Gnopernicus method
for handling the caret will fail for these things
* copy and paste doesn't put them in the clipboard. Somehow that part works for
ordered lists, but I suspect that's hard coded somewhere. The :before and :after
frames' text aren't copied to the clipboard.
* Find in page and find-as-you-type don't see these items.

Perhaps if we used anonymous content instead of just anonymous frames that would
fix some of the problems, or make it easier to. Anyone have a comment?

Comment 4

[Joe Francis]

do not use jfrancis @ in bugzilla.  use [e-mail].

Comment 5

[Aaron Leventhal]

Using viewer.exe, Debug -> Frames we can see what the frame structure looks like
when these anonymous frames are inserted:
Block(h1)
  Inline(h1) 
      Text "Section: "
  Text "Fruits"

So when do we create the accessibles for the anonymous frames?

Comment 6

[Aaron Leventhal]

It turns out that the problem with :before and :after is different.

* For :before and :after, we have to look for anonymous child frames where
frame->IsGeneratedContentFrame() returns true.

* For list item bullets, we have to add something perhaps like this:

nsBlockFrame::GetAccessible(nsIAccessible **aAccessible)
{
if (HaveOutsideBullet()) {
  nsCOMPtr<nsIAccessibilityService> accService =
do_GetService("@mozilla.org/accessibilityService;1");
  if (accService) {
    return accService->CreateBulletAccessible(mBullet, aAccessible);
  }  
}

nsAccessiblityService::CreateBulletAccessible(nsBulletFrame *aBulletFrame,
nsIAccessible **aAccssible)
{
...
}

nsBulletAccessible::GetName(nsAString &aName)
{
  aName.Truncate();
  if (bulletFrame && bulletFrame->GetListItemText(aPresContext,
bulletFrame->GetStyleList(), aName)) {
    return NS_OK;
  }
  return NS_ERROR_FAILURE;
}

    if (childFrame && childFrame->GetType() == nsAccessibilityAtoms::bulletFrame) {
      nsBulletFrame *bulletFrame = NS_STATIC_CAST(nsBulletFrame*, childFrame);

      nsAutoString bulletText;
      if (bulletFrame && bulletFrame->GetListItemText(aPresContext,
frame->GetStyleList(), bulletText)) {

      }
      }
    }

Comment 7

[Aaron Leventhal]

Comment on attachment 147726 [details]
Test case #2

Use the URL listed in the bug summary for a test case.

Comment 8

[Aaron Leventhal]

Attachment: Add lists, list items and bullets to accessibility tree

Works very well with all kinds of list styles.

Seeking r=louie

Comment 9

[Aaron Leventhal]

Comment on attachment 149462 [details] [diff] [review]
Add lists, list items and bullets to accessibility tree 

Louie, please ignore the changes to image and area accessibles. That's part of
a different patch.

Comment 10

[Aaron Leventhal]

Attachment: Same patch but with GetState() implemented for lists, listitems so that they're readonly and not focusable

Comment 11

[Aaron Leventhal]

This is going to require changes in Window-Eyes.

Window-Eyes just reads "listbox listbox listbox listbox listbox listbox" with
the example here:
http://www.corecss.com/examples/15-02-list-style-type_css1-values.html

1. They should be read as "list" not "listbox" when they're readonly and not
focusable
2. Window-Eyes should read their contents.

Comment 12

[Aaron Leventhal]

Louie, I wonder if it's crashing because Mozilla doesn't expose each text node
as an accessible object. You expose each block as an accessible object. Since
the list item is a block, perhaps there's a conflict with the accessible it's
trying to create already.

So, maybe we don't want to do any of this for ATK and have a different solution
for list numbers there (maybe it already works?)

Does it stop crashing if you put 
#ifndef MOZ_ACCESSIBILITY_ATK
#endif
around nsBlockAccessible::GetAccessible() ?

- Aaron

Comment 13

[Louie Zhao]

Comment on attachment 149510 [details] [diff] [review]
Same patch but with GetState() implemented for lists, listitems so that they're readonly and not focusable

No problem for Linux build. Using at-poke, style info before <li> can be
reported.

Comment 14

[Aaron Leventhal]

Comment on attachment 149510 [details] [diff] [review]
Same patch but with GetState() implemented for lists, listitems so that they're readonly and not focusable

Removing the second request. First request for sr= still applies.

Comment 15

[Johnny Stenback (:jst)]

Comment on attachment 149510 [details] [diff] [review]
Same patch but with GetState() implemented for lists, listitems so that they're readonly and not focusable

- In nsHTMLTextAccessible.h

+class nsHTMLListBulletAccessible : public nsHTMLTextAccessible
...
+  nsAutoString mBulletText;

Don't use nsAutoString as a class member, use nsString, or nsCString with utf-8
conversion if you want to save yourself a bit of memory.

sr=jst, but I'd want dbaron to ok the changes to layout here.

Comment 16

[Aaron Leventhal]

Ok, changing nsAutoString to nsString. I'll post a new patch if requested. 

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 149510 [details] [diff] [review]
Same patch but with GetState() implemented for lists, listitems so that they're readonly and not focusable

Just looking at the layout changes:

>Index: layout/html/base/src/nsBlockFrame.cpp
>@@ -74,9 +74,9 @@
> #include "nsGUIEvent.h"
> #include "nsLayoutErrors.h"
> #include "nsAutoPtr.h"
>-#ifdef MOZ_ACCESSIBILITY_ATK
>-#include "nsIAccessibilityService.h"
>+#ifdef ACCESSIBILITY
> #include "nsIServiceManager.h"
>+#include "nsIAccessibilityService.h"
> #endif

Don't ifdef includes unless the file is unavailable otherwise.	See
http://www.mozilla.org/hacking/portable-cpp.html#dont_ifdef_include

>+NS_IMETHODIMP nsBlockFrame::GetAccessible(nsIAccessible** aAccessible)
>+    bulletText.Assign(L'\x2022'); // Unicode bullet character

I doubt this is portable.  Try ...Assign(PRUnichar(0x2022));

With those changes, sr=dbaron on the layout changes.

Comment 18

[Aaron Leventhal]

Checking in accessible/public/nsIAccessibilityService.idl;
/cvsroot/mozilla/accessible/public/nsIAccessibilityService.idl,v  <-- 
nsIAccessibilityService.idl
new revision: 1.43; previous revision: 1.42
done
Checking in accessible/src/base/nsAccessibilityAtomList.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityAtomList.h,v  <-- 
nsAccessibilityAtomList.h
new revision: 1.3; previous revision: 1.2
done
Checking in accessible/src/base/nsAccessibilityService.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityService.cpp,v  <-- 
nsAccessibilityService.cpp
new revision: 1.109; previous revision: 1.108
done
Checking in accessible/src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.100; previous revision: 1.99
done
Checking in accessible/src/base/nsAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsAccessible.h,v  <--  nsAccessible.h
new revision: 1.47; previous revision: 1.46
done
Checking in accessible/src/msaa/nsAccessibleWrap.cpp;
/cvsroot/mozilla/accessible/src/msaa/nsAccessibleWrap.cpp,v  <-- 
nsAccessibleWrap.cpp
new revision: 1.10; previous revision: 1.9
done
Checking in accessible/src/html/nsHTMLTextAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLTextAccessible.cpp,v  <-- 
nsHTMLTextAccessible.cpp
new revision: 1.32; previous revision: 1.31
done
Checking in accessible/src/html/nsHTMLTextAccessible.h;
/cvsroot/mozilla/accessible/src/html/nsHTMLTextAccessible.h,v  <-- 
nsHTMLTextAccessible.h
new revision: 1.25; previous revision: 1.24
done
Checking in accessible/src/xul/nsXULTextAccessible.h;
/cvsroot/mozilla/accessible/src/xul/nsXULTextAccessible.h,v  <-- 
nsXULTextAccessible.h
new revision: 1.12; previous revision: 1.11
done
Checking in layout/html/base/src/nsBlockFrame.cpp;
/cvsroot/mozilla/layout/html/base/src/nsBlockFrame.cpp,v  <--  nsBlockFrame.cpp
new revision: 3.627; previous revision: 3.626
done
Checking in layout/html/base/src/nsBlockFrame.h;
/cvsroot/mozilla/layout/html/base/src/nsBlockFrame.h,v  <--  nsBlockFrame.h
new revision: 3.183; previous revision: 3.182
done
Checking in layout/html/base/src/nsBulletFrame.h;
/cvsroot/mozilla/layout/html/base/src/nsBulletFrame.h,v  <--  nsBulletFrame.h
new revision: 1.23; previous revision: 1.22
done

Comment 19

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381