Steps to Reproduce: 1. Start GOK. 2. Mozilla starts up. In GOK window, click "Menus"->"File"->"Open Web Location". Bug Observation: Open Web Location dialog pops up, but GOK can't operate on the dialog any more. This operation stops gok from working until the dialogue is exited with the actual mouse. the same goes for "upload File" and page setup and print. This happens a lot with Alerts and dialogues. When they are closed, gok behaves as normal, with focus still on Mozilla. These dialogs are "modal dialog", which prevent AtkActionIface->do_action from returning unless closing them. As long as mozilla "modal dialog" shows, gok will "hang".

Comment on attachment 170902 [details] [diff] [review] patch v1 Looks good to me. I assume there is no issue with all of the accessible objects sharing the one nsITimer?

static nsCOMPtr<nsITimer> mDoCommandTimer; There are problems with using a static nsCOMPtr. You can't be sure what modules will still be loaded when it goes away -- it will sometimes go away at a bad time. You can probably do it if you explicitly set it to null in ::ShutdownXPAccessibility which is called before the app goes down.

Why is the timer necessarry at all? If DoCommand isn't working, it seems like a bug somewhere else in Mozilla. DoCommandCallback() should have some comment in it telling us why it's there.

Comment on attachment 170902 [details] [diff] [review] patch v1 static comptrs are an absolute no no. make a new patch. :)

(In reply to comment #4) > Why is the timer necessarry at all? If DoCommand isn't working, it seems like a > bug somewhere else in Mozilla. When nsIXULElement->Click() trigger to open a new Modal dialog, it won't return until closing such dialog. Please refer to http://lxr.mozilla.org/seamonkey/source/embedding/browser/webBrowser/nsIWebBrowserChrome.idl#134 When GOK triggers mozilla to open a new Modal dialog, a11y code will stop at "DoAction()" function and GOK can't get any accessible information for the new popup dialog. That's the reason timer is used. > > DoCommandCallback() should have some comment in it telling us why it's there. I'll add comment for the code if we get agree on this solution. I'll fix the "static nsCOMPtr" issue in new patch.

+ if (!mDoCommandTimer) { + mDoCommandTimer = do_CreateInstance("@mozilla.org/timer;1"); + } + if (mDoCommandTimer) { + mDoCommandTimer->InitWithFuncCallback(DoCommandCallback, + (void*)mDOMNode, 1, + nsITimer::TYPE_ONE_SHOT); } - return NS_ERROR_FAILURE; + return NS_OK; * If I understand correctly, the timer puts it on a separate thread, so the fact that it does not return until the window closes will only cause that thread to pause, not the main thread. Is that correct? Otherwise, how does using a timer solve the problem. * You can try a timer value of 0 instead of 1, which means, do this right away. * Instead of having copy and pasted code, we should just have a member function to call, so we can just do: + return DoCommand();

> * Instead of having copy and pasted code, we should just have a member function > to call, so we can just do: > + return DoCommand(); Or maybe the default impl of DoAction in nsAccessible do this, and just call up to it when DoCommand is what should happen.

updated patch: 1. Define DoCommand function in nsAccessible to avoid copy/paste the same code 2. Set the timer value to 0 instead of 1, it works fine 3. Resolve "static nsCOMPtr issue": The attemp to share mDoCommandTimer among all accessible object will make the code looks ugly. Now define mDoCommandTimer in nsAccessible. It won't cost much memory because the timer instance will be created only when it's used.

I still don't want to take up the extra 4 bytes per node for the timer. You can do it like nsAccessNode::gStringBundle. You just have to use NS_ADDREF and NS_RELEASE in the right places, which really isn't that bad or ugly. http://lxr.mozilla.org/seamonkey/source/accessible/src/base/nsAccessNode.cpp#205

Comment on attachment 171859 [details] [diff] [review] patch v2 Minusing until the timer is a static var.

Louie, can you test this? If it works, take this patch and add the comments it needs. If I can understand why we use the timer, maybe it will be useful in other places besides opening a dialog. It was easier to define the timer in nsAccessNode in terms of shutting it down.

Thanks for help on updating the patch. Based on the new one, the comment below is added. +/* + * Use Timer to execute "Click" command of XUL/HTML element (e.g. menuitem, button...). + * + * When "Click" is to open a "modal" dialog/window, it won't return untill the + * dialog/window is closed. If executing "Click" command directly in + * nsXXXAccessible::DoAction, it will block AT-Tools(e.g. GOK) that invoke + * "action" of mozilla accessibles direclty. + */

Comment on attachment 171965 [details] [diff] [review] patch v3 What is it about the timer event that makes it not block the UI thread. Does the timer event occur on a different thread? > direclty Should be spelled directly

After applying the timer, it will be executed in a seperate thread and won't block the main process. When the modal dialog pops up, GOK can continue to grab the new dialog.

(In reply to comment #15) > After applying the timer, it will be executed in a seperate thread and won't > block the main process. When the modal dialog pops up, GOK can continue to grab > the new dialog. Okay, please add that to the comments in the code. Thank you.

patch checked in