Anonymous frames for :before and :after are not exposed in accessibility

RESOLVED FIXED

Status

()

Core
Disability Access APIs
P1
normal
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: Aaron Leventhal, Assigned: Aaron Leventhal)

Tracking

({access, sec508})

Trunk
x86
Windows XP
access, sec508
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: Looking for sr= from Henry)

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

14 years ago
In documents that use CSS :before and :after, the text for the inserted frames
is not exposed in accessible tree.
(Assignee)

Comment 1

14 years ago
Created attachment 148213 [details]
Testcase
(Assignee)

Comment 2

14 years ago
Louie or Jessie, do you see this problem or the problem in bug 242594 using at-poke?
(Assignee)

Comment 3

14 years ago
Suggested fix:

When walking accessible tree, make sure to create accessible objects for
anonymous frames.

   if (childFrame && (childFrame->IsGeneratedContentFrame()) {
      frame = childFrame;
      childFrame = frame->GetFirstChild(nsnull);
    }
  }

I will be able to do this after the patch for 242589 goes in.
Blocks: 242589
(Assignee)

Comment 4

14 years ago
The frame tree looks something like:

Block(h1)
  Inline(h1) 
      Text "Section: "
  Text "Fruits"
(Assignee)

Comment 5

14 years ago
Created attachment 150030 [details] [diff] [review]
1) Make nsAccessibleTreeWalker walk frames when anonymous frames exist as children of the current dom node's frame 2) Streamline nsAccessibleTreeWalker and remove unused code
(Assignee)

Updated

14 years ago
Attachment #150030 - Flags: superreview?(Henry.Jia)
Attachment #150030 - Flags: review?(Louie.Zhao)

Comment 6

14 years ago
Aron, I have tested the patch on Linux (based on provided testcase). Below are
some info about my testing.

Before applying the patch (using the latest trunk code)

1. All texts wrapped by <h1> (with style "before:...") have no corresponding
at-object, which means that <h1> (with style "before:...") nodes aren't
accessible through at-poke.
2. All texts wrapped by <p> are fine. The content of "After:..." is correctly
exposed.

After applying the patch

1. <h1> (with style "before:...") still can't be exposed.
2. Through at-poke, there are 4 additional "at-object", which are all for the
last <p> node -- "Life is short, eat this first".
(Assignee)

Comment 7

14 years ago
The differences between how we expose text in ATK and MSAA are very different.

I need to get my Linux build going so I can understand the difference better.
This patch definitely improves things for MSAA.

Can we #ifndef MOZ_ACCESSIBILITY_ATK on the frame walking parts for now, so we
can get this in and make it work for ATK later?
(Assignee)

Comment 8

14 years ago
Created attachment 150260 [details] [diff] [review]
Fix for MSAA now. Will fix for ATK later.

Louie,

Can you try this patch with and without the |#ifndef MOZ_ACCESSIBILITY_ATK| in
nsAccessibleTreeWalker::UpdateFrame() ?

I think an extra if () statement messed us up there. If it still doesn't work
let's check it in with the |#ifndef| and we can leave the bug open for the ATK
part of it.
Attachment #150030 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #150030 - Flags: superreview?(Henry.Jia)
Attachment #150030 - Flags: review?(Louie.Zhao)
(Assignee)

Comment 9

14 years ago
Comment on attachment 150260 [details] [diff] [review]
Fix for MSAA now. Will fix for ATK later.

Seeking r=Louie Zhao
Attachment #150260 - Flags: review?(Louie.Zhao)

Comment 10

14 years ago
Comment on attachment 150260 [details] [diff] [review]
Fix for MSAA now. Will fix for ATK later.

Since it has no harm to Linux build, you can fix it for MSAA first.
Attachment #150260 - Flags: review?(Louie.Zhao) → review+
(Assignee)

Updated

14 years ago
Attachment #150260 - Flags: superreview?(Henry.Jia)
(Assignee)

Updated

14 years ago
Whiteboard: Looking for sr= from Henry
(Assignee)

Updated

14 years ago
Priority: -- → P1

Comment 11

14 years ago
Comment on attachment 150260 [details] [diff] [review]
Fix for MSAA now. Will fix for ATK later.

>Index: src/base/nsAccessible.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v
>retrieving revision 1.101
>diff -u -r1.101 nsAccessible.cpp
>--- src/base/nsAccessible.cpp	5 Jun 2004 17:56:59 -0000	1.101
>+++ src/base/nsAccessible.cpp	8 Jun 2004 12:11:59 -0000
>@@ -277,6 +277,15 @@
>     // This node has been shut down
>     return NS_ERROR_FAILURE;
>   }
>+  if (!mParent) {
>+    nsCOMPtr<nsIAccessible> parent;
>+    GetParent(getter_AddRefs(parent));
>+    if (parent) {
>+      PRInt32 numChildren;
>+      GetChildCount(&numChildren);  // Make sure we cache all of the children
>+    }
>+  }
>+
This part of code doesn't do any really work. Right?
(Assignee)

Comment 12

14 years ago
(In reply to comment #11)
> >+  if (!mParent) {
> >+    nsCOMPtr<nsIAccessible> parent;
> >+    GetParent(getter_AddRefs(parent));
> >+    if (parent) {
> >+      PRInt32 numChildren;
> >+      GetChildCount(&numChildren);  // Make sure we cache all of the 
> This part of code doesn't do any really work. Right?

You're right. It's supposed to say |parent->GetChildCount(&numChildren)| not
just GetChildCount(&numChildren)|

This code makes GetNextSibling() always use the cached mNextSibling. If there's
no cached parent yet (e.g. if an accessible is created from an event in the
middle of the document), and GetNextSibling() is called on that, then this code
will make sure we have the cached mNextSibling. Calling
parent->GetChildCount(&numChildren) makes sure that all of the parent's children
are cached via mParent::mFirstChild and mNextSibling on each of the children.

Comment 13

14 years ago
Comment on attachment 150260 [details] [diff] [review]
Fix for MSAA now. Will fix for ATK later.

sr=Henry

Pls make the related change (parent->GetChildCount) before checking in.
Attachment #150260 - Flags: superreview?(Henry.Jia) → superreview+
(Assignee)

Comment 14

14 years ago
Checking in src/base/nsAccessNode.h;
/cvsroot/mozilla/accessible/src/base/nsAccessNode.h,v  <--  nsAccessNode.h
new revision: 1.12; previous revision: 1.11
done
Checking in src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.103; previous revision: 1.102
done
Checking in src/base/nsAccessibleTreeWalker.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibleTreeWalker.cpp,v  <-- 
nsAccessibleTreeWalker.cpp
new revision: 1.5; previous revision: 1.4
done
Checking in src/base/nsAccessibleTreeWalker.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibleTreeWalker.h,v  <-- 
nsAccessibleTreeWalker.h
new revision: 1.4; previous revision: 1.3
done
Checking in src/html/nsHTMLFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLFormControlAccessible.cpp,v  <-- 
nsHTMLFormControlAccessible.cpp
new revision: 1.51; previous revision: 1.50
done
Checking in src/html/nsHTMLTextAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLTextAccessible.cpp,v  <-- 
nsHTMLTextAccessible.cpp
new revision: 1.33; previous revision: 1.32
done
Checking in src/xul/nsXULFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.cpp,v  <-- 
nsXULFormControlAccessible.cpp
new revision: 1.37; previous revision: 1.36
done
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Assignee)

Comment 15

14 years ago
I think this caused bug 248378 -- it fits the regression window. Either we're
doing something wrong or the VC98 compiler is.
(Assignee)

Comment 16

14 years ago
Backing out. I believe this caused bug 248378 becuase of compiler differences.

Checking in src/base/nsAccessNode.h;
/cvsroot/mozilla/accessible/src/base/nsAccessNode.h,v  <--  nsAccessNode.h
new revision: 1.13; previous revision: 1.12
done
Checking in src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.107; previous revision: 1.106
done
Checking in src/base/nsAccessibleTreeWalker.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibleTreeWalker.cpp,v  <-- 
nsAccessibleTreeWalker.cpp
new revision: 1.6; previous revision: 1.5
done
Checking in src/base/nsAccessibleTreeWalker.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibleTreeWalker.h,v  <-- 
nsAccessibleTreeWalker.h
new revision: 1.5; previous revision: 1.4
done
Checking in src/html/nsHTMLFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLFormControlAccessible.cpp,v  <-- 
nsHTMLFormControlAccessible.cpp
new revision: 1.53; previous revision: 1.52
done
Checking in src/html/nsHTMLTextAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLTextAccessible.cpp,v  <-- 
nsHTMLTextAccessible.cpp
new revision: 1.34; previous revision: 1.33
done
Checking in src/xul/nsXULFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.cpp,v  <-- 
nsXULFormControlAccessible.cpp
new revision: 1.39; previous revision: 1.38
done
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 17

14 years ago
Backing this out fixes bug 248378 in

Mozilla 1.8a2
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/20040625

We need to find out what part of this fix doesn't work for VC98
(Assignee)

Comment 18

14 years ago
Rechecked in without bad line:
mState.domNode->GetNextSibling(getter_AddRefs(mState.domNode));
getter_AddRefs() nulls out that arg.

Checking in src/base/nsAccessNode.h;
/cvsroot/mozilla/accessible/src/base/nsAccessNode.h,v  <--  nsAccessNode.h
new revision: 1.14; previous revision: 1.13
done
Checking in src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.108; previous revision: 1.107
done
Checking in src/base/nsAccessibleTreeWalker.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibleTreeWalker.cpp,v  <-- 
nsAccessibleTreeWalker.cpp
new revision: 1.7; previous revision: 1.6
done
Checking in src/base/nsAccessibleTreeWalker.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibleTreeWalker.h,v  <-- 
nsAccessibleTreeWalker.h
new revision: 1.6; previous revision: 1.5
done
Checking in src/html/nsHTMLFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLFormControlAccessible.cpp,v  <-- 
nsHTMLFormControlAccessible.cpp
new revision: 1.54; previous revision: 1.53
done
Checking in src/html/nsHTMLTextAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLTextAccessible.cpp,v  <-- 
nsHTMLTextAccessible.cpp
new revision: 1.35; previous revision: 1.34
done
Checking in src/xul/nsXULFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.cpp,v  <-- 
nsXULFormControlAccessible.cpp
new revision: 1.40; previous revision: 1.39
done
(Assignee)

Comment 19

14 years ago
Fixed, without causing any regressions this time.
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago14 years ago
Resolution: --- → FIXED

Comment 20

13 years ago
Comment on attachment 150260 [details] [diff] [review]
Fix for MSAA now. Will fix for ATK later.

>+    mState.domNode = mState.frame? do_QueryInterface(mState.frame->GetContent()): nsnull;
This only works because of bug 253941 allowing the conversion to
if (mState.frame)
  mState.domNode = do_QueryInterface(mState.frame->GetContent());
else
  mState.domNode = do_QueryInterface(nsnull);

Comment 21

13 years ago
Created attachment 163129 [details] [diff] [review]
Fix nsQueryInterface abuse

Updated

13 years ago
Attachment #163129 - Flags: superreview?(Henry.Jia)
Attachment #163129 - Flags: review?(aaronleventhal)
(Assignee)

Updated

13 years ago
Attachment #163129 - Flags: review?(aaronleventhal) → review+
(Assignee)

Updated

13 years ago
Attachment #163129 - Flags: superreview?(Henry.Jia)
You need to log in before you can comment on or make changes to this bug.