165998 - List item bounds incorrectly exposed via accessibility APIs

Status: RESOLVED FIXED

(Core :: XUL, defect, P1)

Description

[timeless]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1b) Gecko/20020830

tab to the Platform List box in a bugzilla form
You should see the platform field in magnify's window

This affects both html controls (non xbl) and xul controls (address card
properties, mail compose from field, mail compose <to|...> control)

instead you see some random part of the window (I think it's related to where
your mouse was when the gnomes last moved).

Comment 1

[Aaron Leventhal]

I've found that the list item bounds are incorrectly exposed in the following
sitatuons:
1. HTML comboboxes at all times.
2. HTML listboxes when the list has scrolled
3. XUL menulists when the list has scrolled

Comment 2

[Aaron Leventhal]

Attachment: Clean up nsAccessible::GetScreenOrigin(), special case collapsed menulists/combos

It would be nice if GetScreenOrigin() could automatically handle any collapsed
views.

Comment 3

[Aaron Leventhal]

Comment on attachment 155178 [details] [diff] [review]
Clean up nsAccessible::GetScreenOrigin(), special case collapsed menulists/combos

Asking roc for sr= before I get r=, because we need the view expertise first.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  widget->WidgetToScreen(nsRect(origin.x, origin.y, 0, 0), *aRect);

1,1 would be a better width and height ... something might try to be clever with
an empty rectangle.

+  while (selectContent) {
+    if (selectContent->Tag() == nsAccessibilityAtoms::select) {

Probably better to write this as

  while (selectContent && selectContent->Tag() != nsAccessibilityAtoms::select) {
    selectContent = selectContent->GetParent();
  }
  if (!selectContent) {
    return nsAccessible::GetBoundsFrame();
  }
  ...

and similar for the XUL.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 155178 [details] [diff] [review]
Clean up nsAccessible::GetScreenOrigin(), special case collapsed menulists/combos

See comments in the bug

Comment 6

[Aaron Leventhal]

Attachment: 1) Addresses roc's comments 2) Fixes nsHTMLAreaAccessible 3) Gets rid of unnecessary GetScrollingOffsets()

Note, I didn't change the code in nsXULSelectAccessible to use tagname, because
QI is more convenient (there's already more than 1 tag implementing
nsIDOMXULMenuListElement -- menulist and autocomplete).

Comment 7

[Aaron Leventhal]

Comment on attachment 155624 [details] [diff] [review]
1) Addresses roc's comments 2) Fixes nsHTMLAreaAccessible 3) Gets rid of unnecessary GetScrollingOffsets()

Carrying roc's sr=

Comment 8

[Philip K. Warren]

Comment on attachment 155624 [details] [diff] [review]
1) Addresses roc's comments 2) Fixes nsHTMLAreaAccessible 3) Gets rid of unnecessary GetScrollingOffsets()

This code fails to build on Linux, with the following error message:

In file included from
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp:43:
../../../dist/include/dom/nsIDOMXULSelectCntrlEl.h:33: warning: `virtual 
   nsresult 
  
nsIDOMXULSelectControlElement::GetSelectedItem(nsIDOMXULSelectControlItemElemen
t**)
   ' was hidden
../../../dist/include/dom/nsIDOMXULMultSelectCntrlEl.h:73: warning:   by `
   virtual nsresult nsIDOMXULMultiSelectControlElement::GetSelectedItem(int, 
   nsIDOMXULSelectControlItemElement**)'
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp: In instantiation of `nsDerivedSafe<nsIContent>':
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp:337:   instantiated from here
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp:337: error: base
   class `nsIContent' has incomplete type
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp: In
   member function `virtual nsIFrame*
   nsXULSelectOptionAccessible::GetBoundsFrame()':
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp:337: error: no
   matching function for call to `do_QueryInterface(nsCOMPtr<nsIContent>&)'
../../../dist/include/xpcom/nsCOMPtr.h:379: error: candidates are: 
   nsQueryInterface do_QueryInterface(nsISupports*)
../../../dist/include/xpcom/nsCOMPtr.h:386: error:		   
   nsQueryInterfaceWithError do_QueryInterface(nsISupports*, nsresult*)
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp:347: error: no
   matching function for call to `nsDerivedSafe<nsIPresShell>::
   GetPrimaryFrameFor(nsCOMPtr<nsIContent>&, nsIFrame**)'
../../../dist/include/layout/nsIPresShell.h:295: error: candidates are: virtual 
   nsresult nsIPresShell::GetPrimaryFrameFor(nsIContent*, nsIFrame**) const
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp:352: error: `
   GetParent' undeclared (first use this function)
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp:352: error: (Each
   undeclared identifier is reported only once for each function it appears
   in.)
../../../dist/include/xpcom/nsISupportsUtils.h: In static member function `
   static const nsIID& nsCOMTypeInfo<T>::GetIID() [with T = nsIContent]':
../../../dist/include/xpcom/nsCOMPtr.h:571:   instantiated from
`nsCOMPtr<T>::nsCOMPtr(nsQueryInterface) [with T = nsIContent]'
/home/pkw/sb/mozilla/devel-trunk/mozilla/accessible/src/xul/nsXULSelectAccessib
le.cpp:333:   instantiated from here
../../../dist/include/xpcom/nsISupportsUtils.h:202: error: incomplete type '
   nsIContent' cannot be used to name a scope
gmake[3]: *** [nsXULSelectAccessible.o] Error 1
gmake[3]: Leaving directory
`/home/pkw/sb/mozilla/devel-trunk/mozilla/obj-opt/accessible/src/xul'

Comment 9

[Philip K. Warren]

Comment on attachment 155624 [details] [diff] [review]
1) Addresses roc's comments 2) Fixes nsHTMLAreaAccessible 3) Gets rid of unnecessary GetScrollingOffsets()

It looks like this is fixed if you just add an '#include "nsIContent.h"' in
nsXULSelectAccessible.cpp. r=pkw with that change.

Comment 10

[Aaron Leventhal]

Comment on attachment 155624 [details] [diff] [review]
1) Addresses roc's comments 2) Fixes nsHTMLAreaAccessible 3) Gets rid of unnecessary GetScrollingOffsets()

Mozilla 1.8a3 is going to be an important accessibility testing branch for
ZoomText users. Can we get this in? It's the biggest ZoomText bug on our side.
This checkin only affects users testing accessibility software, it doesn't get
loaded otherwise.

I've taken care of pkwarren's comment about the missing #include.

Comment 11

[Aaron Leventhal]

Checking in accessible/src/base/nsAccessibilityAtomList.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityAtomList.h,v  <-- 
nsAccessibilityAtomList.h
new revision: 1.7; previous revision: 1.6
done
Checking in accessible/src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.115; previous revision: 1.114
done
Checking in accessible/src/base/nsAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsAccessible.h,v  <--  nsAccessible.h
new revision: 1.51; previous revision: 1.50
done
Checking in accessible/src/html/nsHTMLAreaAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLAreaAccessible.cpp,v  <-- 
nsHTMLAreaAccessible.cpp
new revision: 1.23; previous revision: 1.22
done
Checking in accessible/src/html/nsHTMLSelectAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLSelectAccessible.cpp,v  <-- 
nsHTMLSelectAccessible.cpp
new revision: 1.38; previous revision: 1.37
done
Checking in accessible/src/html/nsHTMLSelectAccessible.h;
/cvsroot/mozilla/accessible/src/html/nsHTMLSelectAccessible.h,v  <-- 
nsHTMLSelectAccessible.h
new revision: 1.23; previous revision: 1.22
done
Checking in accessible/src/xul/nsXULSelectAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULSelectAccessible.cpp,v  <-- 
nsXULSelectAccessible.cpp
new revision: 1.22; previous revision: 1.21
done
Checking in accessible/src/xul/nsXULSelectAccessible.h;
/cvsroot/mozilla/accessible/src/xul/nsXULSelectAccessible.h,v  <-- 
nsXULSelectAccessible.h
new revision: 1.18; previous revision: 1.17
done

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 155624 [details] [diff] [review]
1) Addresses roc's comments 2) Fixes nsHTMLAreaAccessible 3) Gets rid of unnecessary GetScrollingOffsets()

unsetting 1.8a3 approval request. we've shipped already.

Comment 13

[Firefox Bug Husbandry Bot]

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