417249 - AT-SPI document load complete events not triggered everytime

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[Scott Haeger]

AT-SPI document load complete events are not triggered on every page load.  Notably are some of the Dojo widget test pages linked from http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/form/ .  I am certain the slider and checkbox pages are not triggering the required event and that they did in the past.

Comment 1

[Scott Haeger]

The problem occurred between 0208 (good) and 0209 (bad).

Comment 2

[Marco Zehe (:MarcoZ)]

Aaron, among others, the final fix for bug 412878 falls within that regression range, which directly deals with doc loads. Also candidates are bug 413777, and bug 406010. Other code was checked in on February 8, too, but I don't think they have to do with doc loading/showing/focus issues.
BTW: No problem on Windows whatsoever.

Comment 3

[Ginn Chen]

I can reproduce the bug, if I load the page and press back and forward button.

Yes, it is bug 412878, the change of nsAccessibilityService.cpp
I think document:load-stopped events are also missing.

Where is expected to guarantee the doc accessible creation before the load-complete/stop events?

Comment 4

[Aaron Leventhal]

The doc should be created when the doc load starts.

Comment 5

[Ginn Chen]

On LOAD_START, I found the domDocNode we get from aWebProgress is the old document, not the document being loaded.
So it doesn't creates new doc accessible here.

Comment 6

[Aaron Leventhal]

Marco, does this happen on Windows?

Is it the same as bug 417578.

Comment 7

[Marco Zehe (:MarcoZ)]

(In reply to comment #6)
> Marco, does this happen on Windows?

No.

> Is it the same as bug 417578.

Difficult to say. I've asked in the bug whether the 2008020804 build was still OK (same regression range as this bug). They *might* be related.

Comment 8

[Aaron Leventhal]

Working on this in bug 417828

Comment 9

[Scott Haeger]

Do you mean bug #417578 ?

Comment 10

[Zack Cerza]

I was checking the build referenced at https://bugzilla.mozilla.org/show_bug.cgi?id=412878#c9 to see if it fixed bug 417578, and it seems to, but I'm not seeing any document:load-{stopped,complete} events. I guess I could be doing it wrong, but I am getting object:children-changed events.

Comment 11

[Ginn Chen]

With the patch of bug 412878, I still notice sometimes document:load-complete event is missing.

It seems it is better, but I'm not quite sure.

Comment 12

[Damon Sicore (:damons)]

+'ing w/ P2. 

Comment 13

[Aaron Leventhal]

Attachment: Allow creation of doc accessibles for load complete, as long as content viewer is still present (not after page hide)

Comment 14

[Aaron Leventhal]

Attachment: With other fixes that I think we need (bug 413778, bug 412878 and bug 405951)

I would test with the combined patch, but for this bug the review should just go on the first patch which isolates the fix.

Comment 15

[Aaron Leventhal]

Attachment: Without incorrect document focus changes

Comment 16

[Aaron Leventhal]

Try server builds: https://build.mozilla.org/tryserver-builds/2008-02-25_12:[e-mail]-DocLoadFixes_417249_417828_417578_405951_413778_412878/

Comment 17

[Scott Haeger]

The try server build fails.  The document load complete event is not seen for the Dojo slider.  It doesn't happen at first but after a few tries it will fail to trigger the event.

Comment 18

[Marco Zehe (:MarcoZ)]

Scott, I didn't see the doc load fail with the Slider at all at 5 or more tries. However, i did see spurious doc load finish events for the document I was currently leaving. For example, on the index page, after pressing ENTER on one of the test HTML files, Orca would tell me that it finished loading the index page. It would then interrupt itself to tell me that the slider, checkbox or other page was loaded, and that I was on the heading of that sample page.
With today's regular nightly, I definitely see the doc events not firing, meaning when leaving the index page, orca never tells me that the checkbox example has finished loading.

Do you see those spurious events as well?

Comment 19

[Aaron Leventhal]

Attachment: Put EVENT_INTERNAL_LOAD handling back in

Comment 20

[Scott Haeger]

Marco, no I didn't see any spurious events as you described.  However, I consistently see my problem.  Try this.  First, go to the Dojo index page here http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/form/ and open the slider in a new tab.  Then open the slider in the current tab.  I don't see a load complete event for the new tab one.  This is correct I believe.  The problem is that I also don't see an event for the slider loaded in the current tab.

Comment 21

[Aaron Leventhal]

Attachment: More fixes

This should fix all the current doc load event and doc load crash bugs.

Comment 22

[Aaron Leventhal]

Attachment: Fix shutdown crash

Comment 23

[alexander :surkov (:asurkov)]

Comment on attachment 305632 [details] [diff] [review]
Fix shutdown crash

looks correct with me, please align arguments description of new interface method and initialize used raw pointers by nsnull.

Comment 24

[Aaron Leventhal]

Linux try server builds are here:
https://build.mozilla.org/tryserver-builds/2008-02-25_20:[e-mail]-MoreDocLoadFixes_417249_417828_417578_405951_413778_412878/

Comment 25

[Ginn Chen]

It's not working on my box.

I still don't have document load event for test_ComboBox.html.

Comment 26

[Aaron Leventhal]

Marco saw them, and I just checked in before I saw this. Ginn can you help debug?

Comment 27

[Aaron Leventhal]

Marco & I still think this is fixed. Scott/Ginn/Zack -- more testing from you would be helpful.

Comment 28

[Ginn Chen]

If a nsDocAccessible is created when docShell has BUSY_FLAGS_NONE.
mIsContentLoaded is initialized as PR_TRUE.
Therefore we will not fire document load complete event for it.

Comment 29

[Aaron Leventhal]

Ginn, do you have a consistent way to create that condition (comment 28)?

Comment 30

[Aaron Leventhal]

Attachment: Address Ginn's last scenario, but would still be better to have more details on when it happens. Is it when opening a tab?

Comment 31

[Ginn Chen]

On my machine, open http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/form/
click test_ComboBox.html ( I didn't get load complete )
press back buttion ( I got load complete )
press forward button ( I didn't get load complete )
press reload ( I got document:reload and document:load-complete)

Comment 32

[Ginn Chen]

Comment on attachment 305739 [details] [diff] [review]
Address Ginn's last scenario, but would still be better to have more details on when it happens. Is it when opening a tab?

Works for me now.
Thanks.

Comment 33

[Aaron Leventhal]

Good, but can you look and see why an nsDocAccessible wasn't created with mIsContentLoaded = PR_FALSE for you? It should have created an nsDocAccessible for the nsAccessibilityService::StartLoadCallback() and that doc should still be busy. Was it loaded so fast that after the timeout the docshell is already not busy?

Comment 34

[Ginn Chen]

As I said in Comment #5, in StartLoadCallback, we don't have the right domNode.
At that time, the domNode we get from domWindow is the document being destroyed, not the document being loaded.
I think it's still a bug somewhere, and may cause some problems.
e.g. state-changed:busy is fired for the wrong target on loading start.

In my case, the document is created by "focus" event, and docshell is already not busy by then.

***
And, there's a bug in the patch you committed earlier today.
+  // Fire STATE_CHANGE event for doc load finish if focus is in same doc tree
+  if (gLastFocusedNode) {
This chuck should be inside
 if (isFinished) {
}

Comment 35

[Aaron Leventhal]

Having the wrong dom node at the start load is annoying. This last patch helps a lot in that case, but I'm not sure what to do about it. The timer delay was supposed to help make sure we get the right node for the start load event.

The other part is not bad to fix.

Comment 36

[Aaron Leventhal]

Spun off bug 419626 for wrong DOM node in StartLoadCallback.

Comment 37

[Scott Haeger]

Try server build from comment #24 fails the test that I outlined in comment #20.

Comment 38

[Aaron Leventhal]

As discussed on IRC, at this point that's the wrong build to test.

Comment 39

[Scott Haeger]

The latest tinderbox build dated 26-Feb-2008 07:00 has consistently good document load complete events, even for documents loaded in hidden tabs.