popup menu could not be grabbed by GOK.

RESOLVED WONTFIX

Status

()

RESOLVED WONTFIX
13 years ago
7 years ago

People

(Reporter: tim.miao, Assigned: davidb)

Tracking

(Blocks: 1 bug, {access})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050607 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.9a1) Gecko/20060402 Firefox/1.6a1

Downdrop list menuitems could not be grabbed by GOK.

Reproducible: Always

Steps to Reproduce:
1. Launch firefox and gok.
2. Visit more than 3 different websites.
3. Select UI Grab->Back->Back(Second Back button) to expand downdrop list menu.

Actual Results:  
This downdrop list menu can not be grabbed by GOK.

Expected Results:  
Firefox downdrop list menu should be grabbed by GOK.
(Reporter)

Updated

13 years ago
Keywords: access
OS: Other → Solaris
Hardware: Other → Sun
*** Bug 335198 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Blocks: 333488
the accessible node is created.

I doubt how gok can grab the dynamic node.

Tim, can you give a scenario that gok can grab similar node?
includes right click popup
Summary: Downdrop list menuitems could not be grabbed by GOK. → popup menu could not be grabbed by GOK.

Comment 4

12 years ago
Just a guess, but mozilla doesn't seem to fire a window_activate and for gedit i see one.
at-poke events log shows strange:

trace window->active, object->children-changed, state-changed
1.open a new gnome-terminal(or any gtk/gnome application). right click to popup menu

children changed event (object:children-changed:add) 0 0 parent: A|ap:application:gnome-terminal: child: A|co:frame:neoliu@dizzy:~:
generic event 'object:state-changed:visible' A|co:window::(1) (0)
generic event 'object:state-changed:iconified' A|co:window::(0) (0)
generic event 'object:state-changed:focused' A|co|te:terminal:Terminal:neoliu@dizzy:~(0) (0)
generic event 'object:state-changed:focused' A|co|te:terminal:Terminal:neoliu@dizzy:~(0) (0)
generic event 'object:state-changed:focused' A|co|te:terminal:Terminal:neoliu@dizzy:~(0) (0)
generic event 'object:state-changed:active' A|co:frame:neoliu@dizzy:~:(0) (0)
window event 'window:activate' (0) (0) on A|co:window:: with title ''
generic event 'object:state-changed:active' A|co:window::(1) (0)

there is no menu related event.

2.use THAT terminal and right click to popup again

children changed event (object:children-changed:remove) 0 0 parent: A|co:window:: child: <Null>
generic event 'object:state-changed:defunct' A|co|se:menu::(1) (0)
generic event 'object:state-changed:defunct' A|co:window::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:menu item::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:menu item::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:separator::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:menu item::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:separator::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:menu item::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:menu item::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:separator::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te|se:menu::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:menu item::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:check menu item::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te:separator::(1) (0)
children changed event (object:children-changed:remove) 0 0 parent: A|co|se:menu:: child: <Null>
generic event 'object:state-changed:defunct' A|co|ac|te|se:menu::(1) (0)
children changed event (object:children-changed:add) 0 0 parent: A|ap:application:gnome-terminal: child: A|co:frame:neoliu@dizzy:~:
generic event 'object:state-changed:visible' A|co:window::(1) (0)
generic event 'object:state-changed:iconified' A|co:window::(0) (0)
generic event 'object:state-changed:focused' A|co|te:terminal:Terminal:neoliu@dizzy:~(0) (0)
generic event 'object:state-changed:focused' A|co|te:terminal:Terminal:neoliu@dizzy:~(0) (0)
generic event 'object:state-changed:focused' A|co|te:terminal:Terminal:neoliu@dizzy:~(0) (0)
generic event 'object:state-changed:active' A|co:frame:neoliu@dizzy:~:(0) (0)
window event 'window:activate' (0) (0) on A|co:window:: with title ''
generic event 'object:state-changed:active' A|co:window::(1) (0)

The relevant gok logic is in the source file gok-spy.c (search for 'popup')...  For popups IIRC gok expects to get a window deactivate event on whatever window had focus, and a focus event on something with a menu or menu-item role, if these conditions are met, the expected behaviour is for gok to auto-branch to a new keyboard representing the popup.  (Sorry very busy here)

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
atk_object_get_role returns ATK_ROLE_MENU, then call
atk_object__notify_state_change(accWrap->GetAtkObject(),
                                       ATK_STATE_FOCUSED, TRUE)

gok doesn't get into  gok_spy_process_focus

did I miss something here?
Nian,

Can you confirm there is no "pointer grab" while the popup is posted?  I'm not sure why gok_spy_process_focus would not be entered.

Another thing you might be able to try is at-spi/test/even-listener-test to see if that catches the event.
OS: Solaris → All
Hardware: Sun → All
Created attachment 223129 [details] [diff] [review]
patch

atk_focus_tracker_notify does the trick.
still not sure why atk_object_state_change not work
Attachment #223129 - Flags: review?(aaronleventhal)

Updated

12 years ago
Assignee: nobody → nian.liu
OS: All → Linux

Comment 10

12 years ago
Comment on attachment 223129 [details] [diff] [review]
patch

Please declare parent right when it's used, like:
AtkObject *parent = atk_object_get_parent(accWrap->GetAtkObject());

You're sure it does not leak, correct?

I don't think the parent ATK object will always be the previously activated window. For example, DHTML menus are just in the DOM tree. See http://www.mozilla.org/access/dhtml/spreadsheet
With menus like that the parent could be anything.
So, the code to get the previously activated window may need to up the ancestor chain until it reaches a window, or do something more complex like that. If you do something like that you'll want to call it something besides parent.

Please have Ginn Chen review the next patch because I will be on vacation next week.
Attachment #223129 - Flags: review?(aaronleventhal) → review-
Created attachment 223261 [details] [diff] [review]
patch addressed aaron's comments
Attachment #223261 - Flags: review?(ginn.chen)

Comment 12

12 years ago
Comment on attachment 223261 [details] [diff] [review]
patch addressed aaron's comments

1) if we emit deactivate signal here, when will the window get activate signal?

2) do we need care about EVENT_MENUPOPUPEND?

3) do not use g_signal_emit(obj, g_signal_lookup())
use g_signal_emit_by_name, or split it into 2 lines, see Bug 327530

4) When you need to declare a variable in a switch, put the block in braces, i.e. case { } break;

5) don't use myPtr != nsnull or myPtr == nsnull

6) do add spaces to empty line
Attachment #223261 - Flags: review?(ginn.chen) → review-
Created attachment 223423 [details] [diff] [review]
patchv3

address ginn's comment
Attachment #223129 - Attachment is obsolete: true
Attachment #223261 - Attachment is obsolete: true
Attachment #223423 - Flags: review?(ginn.chen)

Comment 14

12 years ago
I found sometimes atk_object_get_parent(focusedObj) returns 0, and then we fail to send out windowdeactivate event.
e.g. click the down arrow right to location bar repeatedly.

Neo, can you investigate it?

Also we don't want to fire activate signal to mActivatedWindow in EVENT_MENUPOPUPEND case, if we fail to get mActivatedWindow in EVENT_MENUPOPUPSTART case.
Created attachment 224035 [details] [diff] [review]
patch

1. not only make window active, also notify atk that it's focused.
2. clear mActivatedWindow for each popup
Attachment #223423 - Attachment is obsolete: true
Attachment #224035 - Flags: review?(ginn.chen)
Attachment #223423 - Flags: review?(ginn.chen)

Comment 16

12 years ago
Comment on attachment 224035 [details] [diff] [review]
patch

For EVENT_MENUPOPUPEND, should fire focus event to the focused widget instead of mActivatedWindow

I'm not sure if it is good to add another private member to record focused object.
We didn't use atk_get_focus_object in mozilla before.
Attachment #224035 - Flags: review?(ginn.chen) → review-
Created attachment 224154 [details] [diff] [review]
patch

agree, test seems ok.
Attachment #224154 - Flags: review?(ginn.chen)

Updated

12 years ago
Attachment #224154 - Flags: review?(ginn.chen) → review?(aaronleventhal)

Comment 18

12 years ago
Comment on attachment 224154 [details] [diff] [review]
patch

I am still skeptical about this patch. Does it handle nested menu events when there is a sub-sub menu? It looks like the second menupopupstart would fire an extra deactivate for the document object.

Also I would like some comments in the code since it is not obvious to new people what the while() loop does. I'm concerned that as new roles get added for new types of top level objects, the code won't get updated.

Comment 19

12 years ago
Comment on attachment 224154 [details] [diff] [review]
patch

what if the previously focused object has been destroyed before the menu goes away, i think it will crash

Comment 20

12 years ago
Comment on attachment 224154 [details] [diff] [review]
patch

what if the previously focused object has been destroyed before the menu goes away, i think it will crash

Comment 21

12 years ago
Comment on attachment 224154 [details] [diff] [review]
patch

Because of this I recommend using gLastFocusedNode instead of caching, as Ginn also mentioned.
Attachment #224154 - Flags: review?(aaronleventhal) → review-

Comment 22

12 years ago
Note: bug 339237 may be affecting your current results.
(In reply to comment #21)
> (From update of attachment 224154 [details] [diff] [review] [edit])
> Because of this I recommend using gLastFocusedNode instead of caching, as Ginn
> also mentioned.
> 

lxr gLastFocusedNode and find it's set value in nsRootAccessible.cpp 431, 721 and 814. and hence
gLastFocusedNode is not reliable in two scenerio:
1.after popupshows and focus on a menuitem. close popup, crash
2.same as cache in patch. dom node destroyed, close popup, crash

anyway we can get focused node in a certain window? that will solve both cache and gLastFocusedNode issue.
(In reply to comment #18)
> (From update of attachment 224154 [details] [diff] [review] [edit])
> I am still skeptical about this patch. Does it handle nested menu events when
> there is a sub-sub menu? It looks like the second menupopupstart would fire an
> extra deactivate for the document object.
haven't found a test case. however, second popup should be ok since first popup is focused before, second popup only deactive first popup not document obj.
> 
> Also I would like some comments in the code since it is not obvious to new
> people what the while() loop does. I'm concerned that as new roles get added
> for new types of top level objects, the code won't get updated.
> 
via which role and how? 

(In reply to comment #22)
> Note: bug 339237 may be affecting your current results.
> 

i'll see what i can do for that

Comment 26

12 years ago
Nian, what is the rule of what kinds of objects fire window: events?

We are saying that context menus should, but what is the rule and where is it defined? If we don't have a good definition for that rule from somewhere, then how do we really know if we're deactivating the right thing?

Comment 27

12 years ago
I have the nsRootAccessible part of this fix in bug 339237, under review. However, it now uses popuphiding instead of DOMMenuInactive.
Created attachment 228925 [details] [diff] [review]
patch
Attachment #224035 - Attachment is obsolete: true
Attachment #224154 - Attachment is obsolete: true
Attachment #228925 - Flags: review?(aaronleventhal)
Created attachment 228927 [details] [diff] [review]
patch

my file mixed a lot.
Attachment #228925 - Attachment is obsolete: true
Attachment #228927 - Flags: review?(aaronleventhal)
Attachment #228925 - Flags: review?(aaronleventhal)

Comment 30

12 years ago
Comment on attachment 228927 [details] [diff] [review]
patch

Nian, sorry could you ask Ginn to review this one? I need to write this article ...
Attachment #228927 - Flags: review?(aaronleventhal) → review?(ginn.chen)

Comment 31

12 years ago
Comment on attachment 228927 [details] [diff] [review]
patch

avoid using = in if-expression
+        AtkObject* tmp = atk_get_focus_object();
+        if (tmp=GetAtkWindow(tmp))
Why not just
AtkObject* tmp = GtkAtkWindow(atk_get_focus_object());
if (tmp) ...

Same problem
+           AtkObject* obj = tmp->GetAtkObject();
+           if (obj = GetAtkWindow(obj)) {
We also don't want to call tmp->GetAtkObject() twice.

+         rv = NS_OK;
+      }
+
need a "break;" here


I'm not clear about the logic of get focusableNode in EVENT_MENUPOPUPEND.
Attachment #228927 - Flags: review?(ginn.chen) → review-

Updated

12 years ago
Blocks: 342901
No longer blocks: 333488
postpone the fix and look into focus, activate/deactivate related issue in deep
346621 is a super set for this

*** This bug has been marked as a duplicate of 346621 ***
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → DUPLICATE
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Depends on: 348961
Created attachment 234180 [details] [diff] [review]
patch

popup node logic is that try to find the focusable node which popup popups(popup node itself can be any node in a DOM window).

focusable node can be an element node or a Document
Attachment #228927 - Attachment is obsolete: true
Attachment #234180 - Flags: review?(ginn.chen)

Comment 35

12 years ago
I found it quite often fails at
+        focusController->GetPopupNode(getter_AddRefs(popupNode));
+        if (popupNode) {

It's random.
(In reply to comment #35)
> I found it quite often fails at
> +        focusController->GetPopupNode(getter_AddRefs(popupNode));
> +        if (popupNode) {
> 
> It's random.
> 

forward/backward is a menupopup not a popup, so there is no popup node for them. the patch solves the bug description and try to recover the focus as much as possible when popup hide.

there is no perfect way to solve menupopup issue. think about submenus. popup node stack may be a solution. however, it's an questionmark that whether it is worth to implement it.

i suggest to check in the patch.

Comment 37

12 years ago
(In reply to comment #36)
> i suggest to check in the patch.
> 

nope, 
send "window:deactivate" and don't send "window:activate" back will cause a major issue with gok.
also I'm wondering if we should emit "window:activate" for the popup window, too.
Status: REOPENED → ASSIGNED

Updated

12 years ago
Attachment #234180 - Flags: review?(ginn.chen) → review-

Updated

12 years ago
Depends on: 365690

Comment 38

12 years ago
Ginn is right - the AT-SPI and ATK docs indicate that window:activate/window:deactivate determine the "currently active window".  If there is no currently active window (i.e. no window:activate) then the desktop is effectively inaccessible, i.e. this is the situation that occurs when an app which doesn't implement AT-SPI has focus.  So absence of active toplevel implies "inaccessible".

"window:" events should only be fired by toplevel window objects.  In most implementations this means toplevels as seen by the X platform, whether managed or unmanaged.  Popups are usually unmanaged X toplevels, though there may be exceptions.  If a popup window is just another child X window (as opposed to a toplevel which is unmanaged), then "window:" events aren't _strictly_ necessary since the API docs don't explicitly require this.  However in this latter case, GOK is unlikely to react as expected, since its UI logic is mostly built around the concept of the currently active toplevel (which seems sensible).   However GOK does react to "children-changed:add" events which are fired on certain types of containers - those which GOK considers "primary containers".  The container types (i.e. ROLES) which GOK currently recognizes as primary are: ROLE_HTML_CONTAINER, ROLE_SCROLL_PANE, ROLE_FRAME, ROLE_DIALOG, ROLE_ROOT_PANE, and ROLE_APPLICATION.  If a popup child is added to any of the above container types, GOK should see it and react appropriately.  However GOK's logic in the latter case will still differ from the "popup window" case since in the latter case, new content is being _added_ to the existing GUI context which includes the enclosing toplevel, whereas in the popup window case, the old GUI context is being temporarily replaced by a new context consisting only of the popup window.

Updated

12 years ago
Blocks: 368875
No longer blocks: 342901

Comment 39

11 years ago
David, can you look at this and tell us if we still need to do something in Firefox 3, or whether it works now, or what?
Assignee: nian.liu → david.bolter
Status: ASSIGNED → NEW
Component: Disability Access → Disability Access APIs
Product: Firefox → Core
QA Contact: disability.access → accessibility-apis
Working on it.  So far the second back button (see original bug description) is no longer showing up so I'll need to play with accerciser/at-poke to diagnose.  (Noticing other newish gok-ff3 weirdness though... unnecessary "hits" on UI Grab... nothing major... appears GOK thinks some links are editable)
Note the second back button is available to me in alpha 7 and I can confirm the bug remains there. What about exposing the xul back button widget that causes the popup as a menu?  (I'm guessing that would be non-trivial because of the way the popup is built?)
Oh just noticed bug 279703 -- will need to catch up on that work and see what we need.
Depends on: 396941
Update: GOK currently does not see the "second" back button -- the one with the dropdown list.
Gerd can you confirm?
Can you look at this please?
David citing:
Closing as WONTFIX since GOK is stale and it is clear we won't have time to fix
this anyways. (Caribou is the new GOK).
Status: NEW → RESOLVED
Last Resolved: 12 years ago7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.