300782 - No event fired when a frame's src changes, and new content is loaded

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

This is one of the last major blockers for Firefox 1.1 accessibility adoption.

We need to fire an event when just one frame in a frameset changes, so that the
screen reader can reload the page.

Comment 1

[Aaron Leventhal]

Attachment: Fire EVENT_REORDER on frame or iframe object when its content has finished loading

Comment 2

[Aaron Leventhal]

A good example of the problem is at http://www.exactaudiocopy.org -- click on
one of the links in nav bar and the main frame loads new content.

Comment 3

[timeless]

Comment on attachment 189390 [details] [diff] [review]
Fire EVENT_REORDER on frame or iframe object when its content has finished loading

>Index: accessible/src/base/nsAccessibilityService.cpp
>@@ -182,28 +182,28 @@ NS_IMETHODIMP nsAccessibilityService::On
>+    return NS_OK;   // A frames or iframe has begun to load new content

frame

>Index: accessible/src/base/nsDocAccessible.cpp
>@@ -1068,14 +1068,19 @@ NS_IMETHODIMP nsDocAccessible::Invalidat
>+    // Don't go any higher than the root of the content or UI document chain
bz and i aren't sure if this comment can ever be wrong.

>Index: accessible/src/msaa/nsDocAccessibleWrap.cpp
>@@ -404,19 +404,30 @@ void nsDocAccessibleWrap::DocLoadCallbac
>+      // A frames or iframe has finished loading new content

>+    NS_ASSERTION(docShell, "No docShell for docShellTreeItem");
hrm, an assertion i never expect to hit, strange.

>-  docAccessible->FireDocLoadingEvent(!(aStateFlags & STATE_TRANSFERRING));
>-  return NS_OK;
>+  return docAccessible->FireDocLoadingEvent(!(aStateFlags & STATE_START));

out of sleepy curiosity, does your caller care that you're now sometimes maybe
throwing an exception?

Comment 4

[Aaron Leventhal]

(In reply to comment #3)
> >Index: accessible/src/base/nsDocAccessible.cpp
> >@@ -1068,14 +1068,19 @@ NS_IMETHODIMP nsDocAccessible::Invalidat
> >+    // Don't go any higher than the root of the content or UI document chain
> bz and i aren't sure if this comment can ever be wrong.
Can you give more detail?

> 
> >Index: accessible/src/msaa/nsDocAccessibleWrap.cpp
> >@@ -404,19 +404,30 @@ void nsDocAccessibleWrap::DocLoadCallbac
> >+      // A frames or iframe has finished loading new content
> 
> >+    NS_ASSERTION(docShell, "No docShell for docShellTreeItem");
> hrm, an assertion i never expect to hit, strange.
Well, you won't hit it, but it's not a bad thing to put -- self documenting no?

> >-  docAccessible->FireDocLoadingEvent(!(aStateFlags & STATE_TRANSFERRING));
> >-  return NS_OK;
> >+  return docAccessible->FireDocLoadingEvent(!(aStateFlags & STATE_START));
> 
> out of sleepy curiosity, does your caller care that you're now sometimes maybe
> throwing an exception?
Maybe. I'll return NS_OK instead.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 189390 [details] [diff] [review]
Fire EVENT_REORDER on frame or iframe object when its content has finished loading

>Index: accessible/src/base/nsAccessibilityService.cpp
>+    return NS_OK;   // A frames or iframe has begun to load 

"frame"

>Index: accessible/src/base/nsDocAccessible.cpp
>+    // Don't go any higher than the root of the content or UI 
document chain

I'm not sure what this comment is saying....

>+      containerAccessible = this;  // At the root of UI or 
content

And again.  There can be all sorts of docshell trees other than "UI" and
"content", in theory.  In embedding apps especially.

>Index: accessible/src/msaa/nsDocAccessibleWrap.cpp
>+      // A frames or iframe has finished loading new content

"frame"

sr=bzbarsky with the comments fixed up.

Comment 6

[Aaron Leventhal]

Comment on attachment 189390 [details] [diff] [review]
Fire EVENT_REORDER on frame or iframe object when its content has finished loading

Will fix the comments and the return value as asked.

Comment 7

[Aaron Leventhal]

Comment on attachment 189390 [details] [diff] [review]
Fire EVENT_REORDER on frame or iframe object when its content has finished loading

Will fix the comments and the return value as asked.

Comment 8

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

Comment on attachment 189390 [details] [diff] [review]
Fire EVENT_REORDER on frame or iframe object when its content has finished loading

This doesn't look like a security fix or a regression from a security fix, so I
don't see the rationale for the approval-aviary1.0.6 request.  If you meant to
request that, please explain why.

Comment 9

[Aaron Leventhal]

(In reply to comment #8)
> (From update of attachment 189390 [details] [diff] [review] [edit])
> This doesn't look like a security fix or a regression from a security fix, so I
> don't see the rationale for the approval-aviary1.0.6 request.  If you meant to
> request that, please explain why.
> 

No. I hit the wrong flag. Then hit stop on the browser and cleared it and set
the right one and resubmitted.

I guess the second submit with the cleared flag didn't override the first
submit's flag. They combined.

Comment 10

[Aaron Leventhal]

Checking in accessible/src/base/nsAccessibilityService.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityService.cpp,v  <-- 
nsAccessibilityService.cpp
new revision: 1.148; previous revision: 1.147
done
Checking in accessible/src/base/nsDocAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsDocAccessible.cpp,v  <--  nsDocAccessible.cpp
new revision: 1.67; previous revision: 1.66
done
Checking in accessible/src/msaa/nsDocAccessibleWrap.cpp;
/cvsroot/mozilla/accessible/src/msaa/nsDocAccessibleWrap.cpp,v  <-- 
nsDocAccessibleWrap.cpp
new revision: 1.30; previous revision: 1.29
done

Comment 11

[Aaron Leventhal]

Forgot to mark fixed.

Comment 12

[Firefox Bug Husbandry Bot]

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