Delayed creation of a11y tree for slow loading iframe document in background tab

VERIFIED FIXED in mozilla8

Status

()

Core
Disability Access APIs
--
major
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Jamie, Assigned: surkov)

Tracking

(Blocks: 1 bug)

Trunk
mozilla8
x86
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Steps to reproduce:
1. With a recent build of NVDA running (2011.2beta1 is fine), open the URL specified in this bug in Firefox.
2. After the document is focused (but before 10 seconds), open a new tab with control+t.
3. Wait 10 seconds.
4. Close the tab created in step 2.
5. Examine the document in NVDA browse mode.
Expected: The iframe should contain the text "document content".
Actual: The iframe contains the URL of the iframe document.

Deeper investigation shows that:
1. When the iframe document finishes loading (around 10 seconds after the top level document loads), its document accessible gets created and a reorder event is fired on the iframe, which is correct.
2. However, the iframe document has empty accessible text and no children (no a11y tree).
3. Therefore, when NVDA's virtual buffer code renders this iframe in response to the reorder event fired in (2), it finds no children in the document and thus resorts to rendering its value, which is the URL.
4. Between 2 and 10 seconds after this occurs, the a11y tree for the document gets created; i.e. accessible text contains 1 embedded object character and there is 1 child.
5. However, when this (4) happens, no event is fired, so NVDA's virtual buffer code does not know to re-render the iframe document.

Note that this only occurs if the tab is in the background while the iframe document is loading and only if the iframe document accessible wasn't yet created when it was moved to the background.
(Assignee)

Updated

6 years ago
Blocks: 572951
(Assignee)

Updated

6 years ago
Severity: normal → major
(Assignee)

Comment 1

6 years ago
Created attachment 546159 [details] [diff] [review]
patch

part1: make sure reorder is fired after initial tree is created

this doesn't help with testcase because it appears the problem is no mutation events when content appears (that happens after initial tree is created).
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
(Assignee)

Comment 2

6 years ago
Comment on attachment 546159 [details] [diff] [review]
patch

actually it works, tested on wrong build
Attachment #546159 - Flags: review?(trev.saunders)
(Assignee)

Updated

6 years ago
Attachment #546159 - Attachment description: part1 → patch
(Assignee)

Comment 3

6 years ago
changing blocking: not mutation tree problem, it's events delivering problem
Blocks: 472809
No longer blocks: 572951
Attachment #546159 - Flags: review?(trev.saunders) → review+
Flags: in-testsuite?
(Assignee)

Comment 4

6 years ago
Trevor, obviously it's nice to have automated test for this but I'm not sure if we can write it without timing, what leads to random oranges. Do you have ideas how to avoid that? Otherwise it could be that your intestsuite? request will hang forever.
(In reply to comment #4)
> Trevor, obviously it's nice to have automated test for this but I'm not sure
> if we can write it without timing, what leads to random oranges. Do you have
> ideas how to avoid that? Otherwise it could be that your intestsuite?
> request will hang forever.

perhaps use an event queue and when the event fires make sure there actually is a tree.  we could then still take a while to fire the event, but atleast we'd be testing that when its fired we created an accessible tree.  but honestly I wasn't completely sure we can test this either so  just would be nice  but I'll leave it up to you.
(Assignee)

Comment 6

6 years ago
We could add such test (at quick glance I didn't find similar in mochitest) but I afraid it will test nothing since time when we created tree and fire reorder event may be random (because they are not in sync), since usually we don't see this bug in real life (I mean not every iframe document has missed content) then this test can be green if this part of code will be broken again. By adding this test I wouldn't set in-testsuite+ if I want to be honest.

However if you lean towards this test makes sense then I add it, at least because one more test doesn't harm.
(Assignee)

Comment 7

6 years ago
Jamie, does NVDA care that reorder of document container and document load event are fired async?
(Reporter)

Comment 8

6 years ago
(In reply to comment #7)
> Jamie, does NVDA care that reorder of document container and document load
> event are fired async?
No. With regard to document rendering, we don't listen for document load complete events at all. (Btw, if I remember correctly, I didn't see a document load complete event for the iframe document in this test case.)
(Assignee)

Comment 9

6 years ago
(In reply to comment #8)
> (In reply to comment #7)
> > Jamie, does NVDA care that reorder of document container and document load
> > event are fired async?
> No. With regard to document rendering, we don't listen for document load
> complete events at all. (Btw, if I remember correctly, I didn't see a
> document load complete event for the iframe document in this test case.)

they are fired for tab documents, so reorder on tab document container and document loaded events aren't in sync. Btw, in case of multiprocess Firefox reorder event is missed in this case.
(Assignee)

Comment 10

6 years ago
Btw, is it a problem if any other event for iframe document can be fired before reorder event on its container?
(Reporter)

Comment 11

6 years ago
(In reply to comment #9)
> [Document load complete events] are fired for tab documents, so reorder on tab document container and
> document loaded events aren't in sync.
Ah. We don't care about the reorder event for tab document containers.

(In reply to comment #10)
> Btw, is it a problem if any other event for iframe document can be fired
> before reorder event on its container?
No. If we receive an event for an object we don't know about, we ignore it. The reorder event should then cause us to re-render the reordered tree.
(Assignee)

Comment 12

6 years ago
> (In reply to comment #10)
> > Btw, is it a problem if any other event for iframe document can be fired
> > before reorder event on its container?
> No. If we receive an event for an object we don't know about, we ignore it.
> The reorder event should then cause us to re-render the reordered tree.

So if you receive focus for something inside iframe document prior reorder event then focus event is likely ignored?
(Reporter)

Comment 13

6 years ago
(In reply to comment #12)
> So if you receive focus for something inside iframe document prior reorder
> event then focus event is likely ignored?
Err... no. Sorry, I should have been clearer. We only ignore events for the buffer for objects we don't yet know about. However, interaction such as focus is handled separately to buffer rendering. In short, focus events won't be ignored, though the content might not be in the buffer yet. In practice, this is pretty unlikely, since even asnyc events should happen within a fraction of a second.
(Assignee)

Comment 14

6 years ago
(In reply to comment #13)
> (In reply to comment #12)
> > So if you receive focus for something inside iframe document prior reorder
> > event then focus event is likely ignored?
> Err... no. Sorry, I should have been clearer. We only ignore events for the
> buffer for objects we don't yet know about.

Right, I bet you told that already, I just forgot about it.
(Assignee)

Comment 15

6 years ago
Created attachment 546535 [details] [diff] [review]
patch2
Attachment #546159 - Attachment is obsolete: true
(Assignee)

Comment 16

6 years ago
Created attachment 546539 [details] [diff] [review]
patch3
Attachment #546535 - Attachment is obsolete: true
Attachment #546539 - Flags: review?(trev.saunders)
Comment on attachment 546539 [details] [diff] [review]
patch3

R=ME WITH THE TEST DEBUG SWITCH TURNED OFF
Attachment #546539 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 18

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/1bf950fbf8a0
Whiteboard: [inbound]
(Assignee)

Comment 19

6 years ago
landed: http://hg.mozilla.org/mozilla-central/rev/1bf950fbf8a0
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
(Assignee)

Comment 20

6 years ago
Marco, can you work on patch port to aurora after few days of running on trunk please?
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110720 Firefox/8.0a1
Status: RESOLVED → VERIFIED
(Assignee)

Updated

6 years ago
Depends on: 674943
We don't test everything here, but we've improved the situation s...
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.