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)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: aaronlev, Assigned: pilgrim)
References
()
Details
(Keywords: access)
Attachments
(1 file)
|
(deleted),
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.
|
Reporter | |
Comment 1•19 years ago
|
||
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.
|
||
Comment 2•19 years ago
|
||
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...
|
|
Reporter | |
Comment 3•19 years ago
|
||
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)
|
|
Reporter | |
Comment 4•19 years ago
|
||
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)
|
|
Reporter | |
Updated•19 years ago
|
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 5•19 years ago
|
||
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 6•19 years ago
|
||
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?
|
|
Reporter | |
Comment 7•19 years ago
|
||
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 8•19 years ago
|
||
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-
|
Assignee | |
Updated•19 years ago
|
Status: NEW → ASSIGNED
|
|
Assignee | |
Updated•19 years ago
|
Assignee: aaronleventhal → pilgrim
Status: ASSIGNED → NEW
|
||
Comment 9•14 years ago
|
||
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.
Description
•