Last Comment Bug 417249 - AT-SPI document load complete events not triggered everytime
: AT-SPI document load complete events not triggered everytime
Status: VERIFIED FIXED
: access, regression
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86 Linux
: P1 critical (vote)
: mozilla1.9beta4
Assigned To: Aaron Leventhal
:
: alexander :surkov
Mentors:
Depends on: 412878
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-13 07:22 PST by Scott Haeger
Modified: 2008-02-26 07:17 PST (History)
5 users (show)
dsicore: blocking1.9+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Allow creation of doc accessibles for load complete, as long as content viewer is still present (not after page hide) (5.73 KB, patch)
2008-02-25 09:26 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
With other fixes that I think we need (bug 413778, bug 412878 and bug 405951) (25.27 KB, patch)
2008-02-25 09:30 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
Without incorrect document focus changes (23.25 KB, patch)
2008-02-25 12:34 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
Put EVENT_INTERNAL_LOAD handling back in (22.84 KB, patch)
2008-02-25 14:10 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
More fixes (24.01 KB, patch)
2008-02-25 15:17 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
Fix shutdown crash (27.66 KB, patch)
2008-02-25 16:53 PST, Aaron Leventhal
surkov.alexander: review+
Details | Diff | Splinter 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? (2.37 KB, patch)
2008-02-26 05:05 PST, Aaron Leventhal
ginn.chen: review+
Details | Diff | Splinter Review

Description Scott Haeger 2008-02-13 07:22:45 PST
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 2008-02-13 09:21:57 PST
The problem occurred between 0208 (good) and 0209 (bad).
Comment 2 Marco Zehe (:MarcoZ) 2008-02-13 10:04:46 PST
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 2008-02-14 02:34:58 PST
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 2008-02-14 06:46:27 PST
The doc should be created when the doc load starts.
Comment 5 Ginn Chen 2008-02-14 23:58:55 PST
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 2008-02-15 07:03:09 PST
Marco, does this happen on Windows?

Is it the same as bug 417578.
Comment 7 Marco Zehe (:MarcoZ) 2008-02-15 07:07:59 PST
(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 2008-02-15 11:00:41 PST
Working on this in bug 417828
Comment 9 Scott Haeger 2008-02-15 11:16:17 PST
Do you mean bug #417578 ?
Comment 10 Zack Cerza 2008-02-18 10:34:09 PST
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 2008-02-19 03:40:44 PST
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) 2008-02-19 14:26:46 PST
+'ing w/ P2. 
Comment 13 Aaron Leventhal 2008-02-25 09:26:33 PST
Created attachment 305545 [details] [diff] [review]
Allow creation of doc accessibles for load complete, as long as content viewer is still present (not after page hide)
Comment 14 Aaron Leventhal 2008-02-25 09:30:42 PST
Created attachment 305546 [details] [diff] [review]
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 2008-02-25 12:34:50 PST
Created attachment 305570 [details] [diff] [review]
Without incorrect document focus changes
Comment 17 Scott Haeger 2008-02-25 13:50:11 PST
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) 2008-02-25 14:03:51 PST
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 2008-02-25 14:10:18 PST
Created attachment 305593 [details] [diff] [review]
Put EVENT_INTERNAL_LOAD handling back in
Comment 20 Scott Haeger 2008-02-25 14:13:58 PST
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 2008-02-25 15:17:24 PST
Created attachment 305621 [details] [diff] [review]
More fixes

This should fix all the current doc load event and doc load crash bugs.
Comment 22 Aaron Leventhal 2008-02-25 16:53:33 PST
Created attachment 305632 [details] [diff] [review]
Fix shutdown crash
Comment 23 alexander :surkov 2008-02-25 20:04:18 PST
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 25 Ginn Chen 2008-02-26 00:49:19 PST
It's not working on my box.

I still don't have document load event for test_ComboBox.html.
Comment 26 Aaron Leventhal 2008-02-26 00:52:55 PST
Marco saw them, and I just checked in before I saw this. Ginn can you help debug?
Comment 27 Aaron Leventhal 2008-02-26 02:22:41 PST
Marco & I still think this is fixed. Scott/Ginn/Zack -- more testing from you would be helpful.
Comment 28 Ginn Chen 2008-02-26 03:42:45 PST
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 2008-02-26 04:40:06 PST
Ginn, do you have a consistent way to create that condition (comment 28)?
Comment 30 Aaron Leventhal 2008-02-26 05:05:51 PST
Created 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?
Comment 31 Ginn Chen 2008-02-26 05:09:02 PST
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 2008-02-26 05:12:32 PST
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 2008-02-26 05:14:49 PST
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 2008-02-26 05:51:13 PST
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 2008-02-26 06:11:08 PST
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 2008-02-26 06:30:35 PST
Spun off bug 419626 for wrong DOM node in StartLoadCallback.
Comment 37 Scott Haeger 2008-02-26 06:37:39 PST
Try server build from comment #24 fails the test that I outlined in comment #20.
Comment 38 Aaron Leventhal 2008-02-26 06:53:36 PST
As discussed on IRC, at this point that's the wrong build to test.
Comment 39 Scott Haeger 2008-02-26 07:11:25 PST
The latest tinderbox build dated 26-Feb-2008 07:00 has consistently good document load complete events, even for documents loaded in hidden tabs.

Note You need to log in before you can comment on or make changes to this bug.