283482 - MSAA events and objects in wrong HWND for CSS overflow children

Status: RESOLVED FIXED

(Firefox :: Disability Access, defect)

Description

[Aaron Leventhal]

Looking at objects and events with the MSAA SDK Inspect.exe tool, the HWND
reported is always in the window that is owned by the document.

However, when CSS forces the need for a new window (widget), the MSAA objects
and events for that subtree should fall within there.

If we don't, it breaks the focus tracking and OSM of screen readers.

Comment 1

[Aaron Leventhal]

Attachment: Make sure MSAA events & WindowFromAccessibleObject() use correct window

Comment 2

[Philip K. Warren]

Comment on attachment 175481 [details] [diff] [review]
Make sure MSAA events & WindowFromAccessibleObject() use correct window

>+%{C++
>+  virtual nsIFrame* GetFrame() = 0;
>+%}

This doesn't look right for an IDL file. What about:

nsIFrame getFrame();

Or just leaving it in the nsAccessNode header file?

>Index: accessible/src/msaa/nsAccessibleWrap.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/src/msaa/nsAccessibleWrap.cpp,v
>retrieving revision 1.18
>diff -p -u -5 -r1.18 nsAccessibleWrap.cpp
>--- accessible/src/msaa/nsAccessibleWrap.cpp	18 Feb 2005 14:36:27 -0000	1.18
>+++ accessible/src/msaa/nsAccessibleWrap.cpp	24 Feb 2005 22:44:47 -0000
>@@ -165,19 +166,44 @@ STDMETHODIMP nsAccessibleWrap::get_accPa
> {
>   *ppdispParent = NULL;
>   if (!mWeakShell)
>     return E_FAIL;  // We've been shut down
> 
>-  nsCOMPtr<nsIAccessible> xpParentAccessible;
>+  nsIFrame *frame = GetFrame();
>+  nsIAtom *frameType = frame ? nsAccessibilityAtoms::viewportFrame : frame->GetType();

So if frame is null, you will call GetType() on it?

>Index: accessible/src/msaa/nsDocAccessibleWrap.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/src/msaa/nsDocAccessibleWrap.cpp,v
>retrieving revision 1.22
>diff -p -u -5 -r1.22 nsDocAccessibleWrap.cpp
>--- accessible/src/msaa/nsDocAccessibleWrap.cpp	18 Feb 2005 14:36:27 -0000	1.22
>+++ accessible/src/msaa/nsDocAccessibleWrap.cpp	24 Feb 2005 22:44:47 -0000
>@@ -227,11 +196,32 @@ NS_IMETHODIMP nsDocAccessibleWrap::FireT
>   if (role == ROLE_SYSTEM_PANE && aEvent == nsIAccessibleEvent::EVENT_STATE_CHANGE) {
>     // Something on the document has changed
>     // Clear out the cache in this subtree
>   }
> 
>-  HWND hWnd = NS_REINTERPRET_CAST(HWND, mWnd);
>+  HWND hWnd;
>+  hWnd = NS_REINTERPRET_CAST(HWND, mWnd);

Nit: Why did you split this into two lines?

Comment 3

[timeless]

if you must have returns, use [notxpcom] nsIFrame getFrame();

Comment 4

[OstGote!]

*** Bug 283503 has been marked as a duplicate of this bug. ***

Comment 5

[Aaron Leventhal]

Sample scrollable div:
http://www.mozilla.org/access/samples/scroll-spreadsheet.html

Comment 6

[Aaron Leventhal]

Attachment: Use [notxpcom] for GetFrame(). We need this in nsPIAccessible so that we can get to it when we have an nsIAccessible()

Thanks for the [notxpcom] tip timeless.

Comment 7

[Philip K. Warren]

You haven't addressed this comment:

>Index: accessible/src/msaa/nsAccessibleWrap.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/src/msaa/nsAccessibleWrap.cpp,v
>retrieving revision 1.18
>diff -p -u -5 -r1.18 nsAccessibleWrap.cpp
>--- accessible/src/msaa/nsAccessibleWrap.cpp	18 Feb 2005 14:36:27 -0000	1.18
>+++ accessible/src/msaa/nsAccessibleWrap.cpp	24 Feb 2005 22:44:47 -0000
>@@ -165,19 +166,44 @@ STDMETHODIMP nsAccessibleWrap::get_accPa
> {
>   *ppdispParent = NULL;
>   if (!mWeakShell)
>     return E_FAIL;  // We've been shut down
> 
>-  nsCOMPtr<nsIAccessible> xpParentAccessible;
>+  nsIFrame *frame = GetFrame();
>+  nsIAtom *frameType = frame ? nsAccessibilityAtoms::viewportFrame :
frame->GetType();

So if frame is null, you will call GetType() on it?

Comment 8

[Aaron Leventhal]

(In reply to comment #7)
> So if frame is null, you will call GetType() on it?
Yowza! Much better after switching that, thank you. Oy, holy smokes that was
dumb on my part.

Comment 9

[Aaron Leventhal]

Attachment: Switch ? : and fix hwnd assignment nit