fire document load events on iframes too

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla15
access
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
we were asked by AT vendor to do that. So let's patch and see if we get any negative effect from this.
(Assignee)

Comment 1

5 years ago
Marco can you test different screen readers and see if this affects them https://tbpl.mozilla.org/?tree=Try&rev=d823fa033eae
(Assignee)

Comment 2

5 years ago
(In reply to alexander :surkov from comment #1)
> Marco can you test different screen readers and see if this affects them
> https://tbpl.mozilla.org/?tree=Try&rev=d823fa033eae

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.alexander@gmail.com-d823fa033eae/

Comment 3

5 years ago
This has the effect that NVDA starts allowing access to a page much later than previously. A simple test I did:

1. Start this try-server build. about:home is set as my default homepage.
Result: NVDA is in focus mode, because about:home automatically sets focus to the search textbox.
2. Press CTRL+L and enter http://www.marcozehe.de. My blog has a few iFrames to load on the front page because you can then easily share the posts from there.
Result: In a regular build, even before all the iframes were loaded, the main document became accessible, and NVDA switched into browse mode.
In this try build, NVDA waits until the very very last of the iframes has finished loading and only then allows access to its virtual cursor by switching to browse mode.

The user experience is that it feels much slower. Users should not have to wait for every single iframe to finish loading before the main document content becomes available to browse.
(Assignee)

Comment 4

5 years ago
I think we can ask Jamie to fix that if NVDA is unique case. What about other screen readers?

Comment 5

5 years ago
The others wait until everything is loaded and start speaking their page summaries. I tested JAWS and Window-Eyes.

Comment 6

5 years ago
What I meant to say was that they are waiting for the last iframe to be done loading anyway. So there is no difference.

Comment 7

5 years ago
(In reply to alexander :surkov from comment #4)
> I think we can ask Jamie to fix that if NVDA is unique case.
I will investigate this in the coming days, but thinking about it, I don't see how NVDA could be at fault here. This suggests that Firefox is no longer firing an event that it used to fire (either focus or document load on the main document). Additional events shouldn't hurt us, only missing ones. One possibility is that document load on the main document is being delayed to wait for document load on the others, but this change doesn't make sense to me. It's a huge backwards compatibility break.
(Assignee)

Comment 8

5 years ago
(In reply to James Teh [:Jamie] from comment #7)
> (In reply to alexander :surkov from comment #4)
> > I think we can ask Jamie to fix that if NVDA is unique case.
> I will investigate this in the coming days, but thinking about it,

thank you, Jamie. Honestly I'm surprised of this effect of the patch.

>  I don't
> see how NVDA could be at fault here. This suggests that Firefox is no longer
> firing an event that it used to fire (either focus or document load on the
> main document).

it shouldn't be a case.

> Additional events shouldn't hurt us, only missing ones. One
> possibility is that document load on the main document is being delayed to
> wait for document load on the others, but this change doesn't make sense to
> me. 

that's right, we do this now and the patch didn't change this.

>It's a huge backwards compatibility break.

Yes. We can introduce some compatibility code to handle that.

Comment 9

5 years ago
(In reply to James Teh [:Jamie] from comment #7)
> One possibility is that document load on the main document is being delayed to
> wait for document load on the others, but this change doesn't make sense to
> me. It's a huge backwards compatibility break.
Please disregard this; I had a brain fail. Document load just shouldn't affect NVDA at all after Gecko 2 because we always render documents as soon as they get focus.

Comment 10

5 years ago
I can't reproduce Marco's observation with this test case:
http://nvda.sourceforge.net/webTests/busy.html
(I prefer this because you can predict when the iframe will finish loading.)
For me, the main document renders straight away and the iframe content renders when it finishes loading 10 seconds later.

Marco, with your test case, what does NVDA say when you press NVDA+tab while it is loading before it switches to browse mode?
(In reply to James Teh [:Jamie] from comment #10)
> I can't reproduce Marco's observation with this test case:
> http://nvda.sourceforge.net/webTests/busy.html
> (I prefer this because you can predict when the iframe will finish loading.)
> For me, the main document renders straight away and the iframe content
> renders when it finishes loading 10 seconds later.

Yes, I'm observing that with your testcase, too.

> Marco, with your test case, what does NVDA say when you press NVDA+tab while
> it is loading before it switches to browse mode?

Nothing, it basically becomes unresponsive until all iframes have finished loading. Then, it executes the NVDA+Tab and tells me the document accessible has focus.

I have the WebVisum extension installed, maybe that makes a difference.

Comment 12

5 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #11)
> > Marco, with your test case, what does NVDA say when you press NVDA+tab while
> > it is loading before it switches to browse mode?
> Nothing, it basically becomes unresponsive until all iframes have finished
> loading.
I did observe nasty responsiveness problems while loading marcozehe.de with that try build, but wasn't sure if it was just me. Alex, could your patch cause severely degraded performance/freezing (as opposed to missing events or the like)?
(Assignee)

Comment 13

5 years ago
(In reply to James Teh [:Jamie] from comment #12)
> (In reply to Marco Zehe (:MarcoZ) from comment #11)
> > > Marco, with your test case, what does NVDA say when you press NVDA+tab while
> > > it is loading before it switches to browse mode?
> > Nothing, it basically becomes unresponsive until all iframes have finished
> > loading.
> I did observe nasty responsiveness problems while loading marcozehe.de with
> that try build, but wasn't sure if it was just me. Alex, could your patch
> cause severely degraded performance/freezing (as opposed to missing events
> or the like)?

It shouldn't be. It makes us fire more events and it shouldn't affect on performance. Let me file new try server build just in case.
(Assignee)

Comment 14

5 years ago
new try server build http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.alexander@gmail.com-d4f858233538/
Same result with this new try-server build. On my blog, NVDA is blocked until *all* iframes have also been loaded. And I'm seeing the performance lag that Jamie describes.
(Assignee)

Comment 16

5 years ago
Here's a new build http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.alexander@gmail.com-b60861c2bfcd/

it fires document loading events for top document of the document load chain. Marco, can you try it please?
No better, NVDA still waits until the very last of those iFrames has finished loading on my blog.
(Assignee)

Comment 18

5 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #17)
> No better, NVDA still waits until the very last of those iFrames has
> finished loading on my blog.

I wonder if you can see a different picture on trunk (because there's no any change in case of your blog: events are preserved)
OK, I can actually get NVDA to speak while the iFrames are still loading. So this latest build *is* better than the previous ones.
(Assignee)

Comment 20

5 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #19)
> OK, I can actually get NVDA to speak while the iFrames are still loading. So
> this latest build *is* better than the previous ones.

actually it should be the same to nightly in case of the web page loading (for example, your blog). If it's not then we need to ask Jamie to take a look.
(Assignee)

Comment 21

5 years ago
So, Marco, can you any negative effect on any screen reader running the latest build?
(Assignee)

Comment 22

5 years ago
Created attachment 627706 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #627706 - Flags: review?(trev.saunders)
Attachment #627706 - Flags: feedback?(marco.zehe)
Comment on attachment 627706 [details] [diff] [review]
patch

>diff --git a/accessible/src/generic/DocAccessible-inl.h b/accessible/src/generic/DocAccessible-inl.h
>new file mode 100644
>--- /dev/null
>+++ b/accessible/src/generic/DocAccessible-inl.h
>@@ -0,0 +1,19 @@
>+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>+/* vim: set ts=2 et sw=2 tw=80: */
>+/* This Source Code Form is subject to the terms of the Mozilla Public
>+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
>+
>+#ifndef mozilla_a11y_DocAccessible_inl_h_
>+#define mozilla_a11y_DocAccessible_inl_h_
>+
>+#include "DocAccessible.h"
>+#include "nsAccessibilityService.h"
>+
>+inline DocAccessible*
>+DocAccessible::ParentDocument() const

shouldnt you include nsIDocument.h too?

loading state.
>+  if (parentTreeItem)
>+    return ParentDocument()->HasLoadState(eCompletelyLoaded);

kind of funny we only fire events when outer doc is loading, ubt I guess we have to keep from breaking people :/

btw, update the comment for this method?
Attachment #627706 - Flags: review?(trev.saunders) → review+
Comment on attachment 627706 [details] [diff] [review]
patch

>+     * Load the document having sub document. No document loading events for
>+     * nested document.

Why? isn't this bug about exactly that, firing those events on iframes, too?
(Assignee)

Comment 25

5 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #24)

> >+     * Load the document having sub document. No document loading events for
> >+     * nested document.
> 
> Why? isn't this bug about exactly that, firing those events on iframes, too?

not exactly, we fire events on root of tree of loading documents
(Assignee)

Comment 26

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #23)

> >+#include "DocAccessible.h"
> >+#include "nsAccessibilityService.h"
> >+
> >+inline DocAccessible*
> >+DocAccessible::ParentDocument() const
> 
> shouldnt you include nsIDocument.h too?

I guess it's included by something else.

> loading state.
> >+  if (parentTreeItem)
> >+    return ParentDocument()->HasLoadState(eCompletelyLoaded);
> 
> kind of funny we only fire events when outer doc is loading, ubt I guess we
> have to keep from breaking people :/

it's ok if you keep them as about coalesced events

> btw, update the comment for this method?

sure
Comment on attachment 627706 [details] [diff] [review]
patch

OK, since the last try-server build doesn't show any negative impact on common Windows screen readers, f=me.
Attachment #627706 - Flags: feedback?(marco.zehe) → feedback+

Comment 28

5 years ago
From Frank DiPalermo on 5/29/2012:
We are wondering if this fix might be also put into the 10.0.x ESR stream?  That is the version that IBM is standardized on and this fix is very important to some IBM applications.
(Assignee)

Comment 29

5 years ago
(In reply to Frank DiPalermo from comment #28)
> From Frank DiPalermo on 5/29/2012:
> We are wondering if this fix might be also put into the 10.0.x ESR stream? 
> That is the version that IBM is standardized on and this fix is very
> important to some IBM applications.

I doubt because only security fixes are allowed on ESR afaik.

Anyway, can we cc somebody from ESR team to answer the question?
(Assignee)

Comment 30

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4c563086f1f
Flags: in-testsuite+
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/b4c563086f1f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 760038
You need to log in before you can comment on or make changes to this bug.