Closed Bug 308560 Opened 19 years ago Closed 14 years ago

Screen reader should either not load page that is refreshing immediately, or at least indicate a refresh is occuring

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: aaronlev, Assigned: pilgrim)

References

(
URL
)

Details

(Keywords: access)

Attachments

(1 file)

Patch for discussion
1.97 KB, patch
Biesinger
: review-
darin.moz
: superreview-
Details | Diff | Splinter Review 
Steps:
Go to www.bt.com with screen reader running

What happens:
Screen reader loads blank page before loading main page, giving the appearance
of an error.
The example at http://www.bt.com starts out with no <body>
It also doesn't fire nsIWebProgressListener with STATE_STOP for the immediately
refreshing blank page.
that you get no STATE_STOP in this case seems like a bug to me...

maybe nsIRefreshURI should have a way to query the pending refreshes...
Let's get Darin's input on whether that's a bug or not.

If there is an immediate refresh set for the page, it's likely to be just a
blank page or content that is not important to read. It is confusing to a
screen reader user because they think the page has finished loading and they
don't have the right content.

I'm not sure what the right fix is. Perhaps the screen reader just needs to
notify the user of refreshes somehow, but we don't currently tell them when a
load event is for an automatic refresh.
Attachment #196155 - Flags: superreview?(darin)
Comment on attachment 196155 [details] [diff] [review]
Patch for discussion

Biesi, I actually like this patch. Here's why:
I let the web progress listener handle it in this special case. If web progress
listener gets fixed to fire these events, I'm still better off than now because
I will get equal numbers of page load start and stop events. I either want a
start and stop for the intermediate page or no events for it.

As things stand now, I am not firing nested page load start/stop events for
this case. I fire start, stop, stop. So, the screen reader does not have a
chance to tell the screen reader that a new page is coming.
Attachment #196155 - Flags: review?(cbiesinger)
Summary: Screen reader should not load page that is refreshing immediately → Screen reader should either not load page that is refreshing immediately, or at least indicate a refresh is occuring
Comment on attachment 196155 [details] [diff] [review]
Patch for discussion

>Index: accessible/src/base/nsRootAccessible.cpp

>@@ -386,10 +392,28 @@ void nsRootAccessible::TryFireEarlyLoadE

First of all, I'm not the best person to review this code because
I really don't know what this code is trying to do.  I'd really like
to see some comments above the newly added section of code explaining
why it exists.	Perhaps it'd be enough to move the comment near the
return statement up above the assignment into docShell.


>+  nsCOMPtr<nsIDocShell> docShell = do_QueryInterface(treeItem);
>+  NS_ASSERTION(docShell, "No docshell for docshelltreeitem");
>+  if (docShell) {

You are asserting that docShell will never be null, so then why not
early return if it is?	Does it really make sense to proceed if you
don't have a docShell?


>+    PRUint32 loadType;
>+    docShell->GetLoadType(&loadType);
>+    // XXX Should have the constants we need for the use of nsIDocShell: GetLoadType()
>+    // on nsIDocShell, not hidden away inside nsDocShell

Please file a bug and mention the bug number in these comments.


>+    const PRUint32 kIgnoreLoadFlags =
>+                   (nsIWebNavigation::LOAD_FLAGS_REPLACE_HISTORY | 
>+                    nsIWebNavigation::LOAD_FLAGS_IS_REFRESH) << 16;
>+    if (loadType & kIgnoreLoadFlags) {
>+      // Use web progress listener for immediate refreshes, don't use early load
>+      // because we don't want the screen reader to read content that
>+      // is really more of a redirect than anything.

It might be good to point out in your comments ways in which this can
occur.	For example, you might mention that this could happen when a
META refresh is processed.  Doesn't this also happen when the user
presses the reload button?
Attachment #196155 - Flags: superreview?(darin) → superreview-
Comment on attachment 196155 [details] [diff] [review]
Patch for discussion

Hm... would it be sufficient to check "Is a document currently loading"? It
seems to me that any time a document is already loading by the time
DOMContentLoaded fires, you don't want to have it read by the screen reader.

hm... also, won't this be wrong if DOMContentLoaded is fired before the refresh
actually happens? For example, if an image on the page takes a while to load?
How would i check to see if the doc is 'still loading' -- it is still marked as
such after DOMContentLoaded because images are still coming in.
Comment on attachment 196155 [details] [diff] [review]
Patch for discussion

ok, since I believe that this WPL behaviour is a bug anyway, I'll minus this -
I'd rather not have this code rely on a bug.

Would it be sufficient to expose the pending timeouts on the docshell (via
nsIRefreshURI)?
Attachment #196155 - Flags: review?(cbiesinger) → review-
Status: NEW → ASSIGNED
Assignee: aaronleventhal → pilgrim
Status: ASSIGNED → NEW
Blocks: keya11y
No longer blocks: keya11y
Depends on: 566103
worksforme per bug 566103 comment #25.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: