277888 - GOK can't work with mozilla modal dialog

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Louie Zhao]

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 1

[Louie Zhao]

Attachment: patch v1

Comment 2

[Philip K. Warren]

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?

Comment 3

[Aaron Leventhal]

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.

Comment 4

[Aaron Leventhal]

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 5

[timeless]

Comment on attachment 170902 [details] [diff] [review]
patch v1

static comptrs are an absolute no no. make a new patch. :)

Comment 6

[Louie Zhao]

(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.

Comment 7

[Aaron Leventhal]

+    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();

Comment 8

[Aaron Leventhal]

> * 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.

Comment 9

[Louie Zhao]

Attachment: patch v2

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.

Comment 10

[Aaron Leventhal]

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 11

[Aaron Leventhal]

Comment on attachment 171859 [details] [diff] [review]
patch v2

Minusing until the timer is a static var.

Comment 12

[Aaron Leventhal]

Attachment: Closer. Still needs comments explaining why we need the timer -- I still don't fully understand how it solves the problem.

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.

Comment 13

[Louie Zhao]

Attachment: patch v3

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 14

[Aaron Leventhal]

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

Comment 15

[Louie Zhao]

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.

Comment 16

[Aaron Leventhal]

(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.

Comment 17

[Louie Zhao]

patch checked in