360865 - Firefox crashes when selecting downloaded item with gok [@ nsXULMenuitemAccessible::DoAction]

Status: RESOLVED FIXED

(Firefox :: Disability Access, defect)

Description

[Tim Miao]

This bug makes firefox crashes.
Steps to reproduce:
1. Launch firefox and gok.
2. Download several files with firefox.
3. Close the download manager.
4. Select the Menu->Tools->Downloads in gok.
5. Select UI Grab in gok, click list button. Select one downloaded item except selected one.

Bug observations:
Firefox crashes after that.

Additional info:
Attached core stack below:
bash-3.00$ pstack core
core 'core' of 1538:    Desktop/firefox/firefox-bin
-----------------  lwp# 1 / thread# 1  --------------------
 d2758cf5 _lwp_kill (1, b) + 15
 d2712162 raise    (b) + 22
 08592211 __1cNnsProfileLockSFatalSignalHandler6Fi_v_ (b, 0, 8046524) + 9d
 d27577ef __sighndlr (b, 0, 8046524, 8592174) + f
 d274d01b call_user_handler (b, 0, 8046524) + 2b8
 d274d1c2 sigacthandler (b, 0, 8046524) + c2
 --- called from signal handler with signal 11 (SIGSEGV) ---
 08ea4429 __1cXnsXULMenuitemAccessibleIDoAction6MC_I_ (aaaf3a0, 0) + a1
 08e96c8c doActionCB (a273c78, 0) + 34
 cf7d769f atk_action_do_action (a273c78, 0) + 47
 ce59e95f impl_doAction (aaacb94, 0, 80469bc) + 27
 ce59b584 _ORBIT_skel_small_Accessibility_Action_doAction (aaacb94, 8046850, 8046840, 8046870, 80469bc, ce59e938) + 18
 ce98112a ORBit_POAObject_invoke (a3a62a8, 8046850, 8046840, 8046870, 80468f8, 80469bc) + 22
 ce985519 ORBit_OAObject_invoke (a3a62a8, 8046850, 8046840, 8046870, 80468f8, 80469bc) + 21
 ce972f1b ORBit_small_invoke_adaptor (a3a62a8, 9e5b400, ce5c06b0, 80468f8, 80469bc) + 2b3
 ce981562 ORBit_POAObject_handle_request (a3a62a8, a1c7104, 0, 0, 0, 9e5b400) + 32a
 ce981960 ORBit_POAObject_invoke_incoming_request (a3a62a8, 9e5b400, 80469bc) + 54
 ce981de6 ORBit_POA_handle_request (9b6c300, 9e5b400, 9e5b418) + 2ea
 ce985317 ORBit_handle_request (979fb38, 9e5b400) + 4b
 ce97014a giop_connection_handle_input (9b6f348) + 2e2
 ce98b4e9 link_connection_io_handler (0, 1, 9b6f348) + 55
 ce98d10d link_source_dispatch (9cb4678, ce98b494, 9b6f348) + 41
 d1bd3615 g_main_dispatch (9786860) + 1d9
 d1bd4705 g_main_context_dispatch (9786860) + 85
 d1bd4b22 g_main_context_iterate (9786860, 1, 1, 9771720) + 3ce
 d1bd4d7b g_main_context_iteration (9786860, 1) + 87
 087e25b0 __1cKnsAppShellWProcessNextNativeEvent6Mi_i_ (9b126c8, 1) + 10
 087c77d2 __1cOnsBaseAppShellYDoProcessNextNativeEvent6Mi_i_ (9b126c8, 1) + 1e
 087c7938 __1cOnsBaseAppShellSOnProcessNextEvent6MpnRnsIThreadInternal_iI_I_ (9b126c8, 9789348, 1, 0) + 98
 d256c6bd __1cInsThreadQProcessNextEvent6Mipi_I_ (9789348, 1, 8046cd4) + 7d
 d252344a __1cVNS_ProcessNextEvent_P6FpnJnsIThread_i_i_ (9789348, 1) + 3a
 087c7806 __1cOnsBaseAppShellDRun6M_I_ (9b126c8) + 2a
 08f0e2c5 __1cMnsAppStartupDRun6M_I_ (9b54b10) + 2d
 08588591 XRE_main (1, 8046ff0, 95a8b48) + 1c59
 085822da main     (1, 8046ff0, 8046ff8) + 16
 08582236 _start   (1, 8047178, 0, 9054f0f, 9054f27, 8047224) + 7a
-----------------  lwp# 2 / thread# 2  --------------------
 d2758785 __pollsys (cedddb50, 1, 0, 0) + 15
 d270f812 poll     (cedddb50, 1, ffffffff) + 52
 d1f61b84 ???????? (980eed0, 1, ffffffff)
 d1f61d40 PR_Poll  (980eed0, 1, ffffffff) + 14
 085e0bb8 __1cYnsSocketTransportServiceEPoll6MipI_i_ (980e9f0, 1, ceddde00) + 94
 085e1415 __1cYnsSocketTransportServicePDoPollIteration6Mi_I_ (980e9f0, 1) + 165
 085e107a __1cYnsSocketTransportServiceSOnProcessNextEvent6MpnRnsIThreadInternal_iI_I_ (980e9f0, 9805000, 1, 1) + 4a
 d256c6bd __1cInsThreadQProcessNextEvent6Mipi_I_ (9805000, 1, cedddeb0) + 7d
 d252344a __1cVNS_ProcessNextEvent_P6FpnJnsIThread_i_i_ (9805000, 1) + 3a
 085e1258 __1cYnsSocketTransportServiceDRun6M_I_ (980e9f0) + 1b4
 d256c72f __1cInsThreadQProcessNextEvent6Mipi_I_ (9805000, 1, cedddf60) + ef
 d252344a __1cVNS_ProcessNextEvent_P6FpnJnsIThread_i_i_ (9805000, 1) + 3a
 d256bc4b __1cInsThreadKThreadFunc6Fpv_v_ (9805000) + 117
 d1f62ec7 ???????? (9805190)
 d2757494 _thr_setup (d1a12400) + 52
 d27576f0 _lwp_start (d1a12400, 0, 0, 0, 0, 0)
-----------------  lwp# 3 / thread# 3  --------------------
 d2757749 __lwp_park (9790bdc, 979b678, ceb7ddf8) + 19
 d2751962 cond_wait_queue (9790bdc, 979b678, ceb7ddf8, 0) + 3e
 d2751cf7 cond_wait_common (9790bdc, 979b678, ceb7ddf8) + 1e1
 d2751f1c _cond_timedwait (9790bdc, 979b678, ceb7de70) + 4a
 d2751fab cond_timedwait (9790bdc, 979b678, ceb7de70) + 27
 d2751fe8 pthread_cond_timedwait (9790bdc, 979b678, ceb7de70) + 21
 d1f5de1c ???????? (9790bdc, 979b678, 5c939)
 d1f5dff2 PR_WaitCondVar (9790bd8, 5c939) + 66
 d2570916 __1cLTimerThreadDRun6M_I_ (979b5b8) + e2
 d256c72f __1cInsThreadQProcessNextEvent6Mipi_I_ (9831288, 1, ceb7df60) + ef
 d252344a __1cVNS_ProcessNextEvent_P6FpnJnsIThread_i_i_ (9831288, 1) + 3a
 d256bc4b __1cInsThreadKThreadFunc6Fpv_v_ (9831288) + 117
 d1f62ec7 ???????? (9831418)
 d2757494 _thr_setup (cea50000) + 52
 d27576f0 _lwp_start (cea50000, 0, 0, 0, 0, 0)
-----------------  lwp# 6 / thread# 6  --------------------
 d2757749 __lwp_park (9824164, 980f8c0, ce56de68) + 19
 d2751962 cond_wait_queue (9824164, 980f8c0, ce56de68, 0) + 3e
 d2751cf7 cond_wait_common (9824164, 980f8c0, ce56de68) + 1e1
 d2751f1c _cond_timedwait (9824164, 980f8c0, ce56dee0) + 4a
 d2751fab cond_timedwait (9824164, 980f8c0, ce56dee0) + 27
 d2751fe8 pthread_cond_timedwait (9824164, 980f8c0, ce56dee0) + 21
 d1f5de1c ???????? (9824164, 980f8c0, 5b8d80)
 d1f5dff2 PR_WaitCondVar (9824160, 5b8d80) + 66
 085ebeac __1cOnsHostResolverPGetHostToLookup6MppnMnsHostRecord__i_ (980f858, ce56df94) + 74
 085ec18f __1cOnsHostResolverKThreadFunc6Fpv_v_ (980f858) + 2f
 d1f62ec7 ???????? (9db84c8)
 d2757494 _thr_setup (cea50800) + 52
 d27576f0 _lwp_start (cea50800, 0, 0, 0, 0, 0)
-----------------  lwp# 13 / thread# 13  --------------------
 d2757749 __lwp_park (9804bbc, 9c001b4, ce42dda8) + 19
 d2751962 cond_wait_queue (9804bbc, 9c001b4, ce42dda8, 0) + 3e
 d2751cf7 cond_wait_common (9804bbc, 9c001b4, ce42dda8) + 1e1
 d2751f1c _cond_timedwait (9804bbc, 9c001b4, ce42de20) + 4a
 d2751fab cond_timedwait (9804bbc, 9c001b4, ce42de20) + 27
 d2751fe8 pthread_cond_timedwait (9804bbc, 9c001b4, ce42de20) + 21
 d1f5de1c ???????? (9804bbc, 9c001b4, 23c34600)
 d1f5dff2 PR_WaitCondVar (9804bb8, 23c34600) + 66
 d1f5e2cc PR_Wait  (9c001b0, 23c34600) + 38
 d256ea38 __1cMnsThreadPoolDRun6M_I_ (9c000b8) + 118
 d256c72f __1cInsThreadQProcessNextEvent6Mipi_I_ (a12c350, 1, ce42df60) + ef
 d252344a __1cVNS_ProcessNextEvent_P6FpnJnsIThread_i_i_ (a12c350, 1) + 3a
 d256bc4b __1cInsThreadKThreadFunc6Fpv_v_ (a12c350) + 117
 d1f62ec7 ???????? (a12c4e0)
 d2757494 _thr_setup (cea50c00) + 52
 d27576f0 _lwp_start (cea50c00, 0, 0, 0, 0, 0)
-----------------  lwp# 14 / thread# 14  --------------------
 d2757749 __lwp_park (a13e354, a12bda8, 0) + 19
 d2751962 cond_wait_queue (a13e354, a12bda8, 0, 0) + 3e
 d2751e4d _cond_wait (a13e354, a12bda8) + 69
 d2751e8b cond_wait (a13e354, a12bda8) + 24
 d2751ec5 pthread_cond_wait (a13e354, a12bda8) + 1e
 d1f5e005 PR_WaitCondVar (a13e350, ffffffff) + 79
 086e8a00 ???????? (1, a123070, d25b31dc, ce22defc, d1f63536, ce22dec4)
 086e91d0 __1cQAsyncWriteThreadDRun6M_I_ (a0a5eb0) + 14
 d256c72f __1cInsThreadQProcessNextEvent6Mipi_I_ (a123070, 1, ce22df60) + ef
 d252344a __1cVNS_ProcessNextEvent_P6FpnJnsIThread_i_i_ (a123070, 1) + 3a
 d256bc4b __1cInsThreadKThreadFunc6Fpv_v_ (a123070) + 117
 d1f62ec7 ???????? (a1228c0)
 d2757494 _thr_setup (cea51400) + 52
 d27576f0 _lwp_start (cea51400, 0, 0, 0, 0, 0)
-----------------  lwp# 17 / thread# 17  --------------------
 d2757749 __lwp_park (a304b64, a312b40, ce77de78) + 19
 d2751962 cond_wait_queue (a304b64, a312b40, ce77de78, 0) + 3e
 d2751cf7 cond_wait_common (a304b64, a312b40, ce77de78) + 1e1
 d2751f1c _cond_timedwait (a304b64, a312b40, ce77def0) + 4a
 d2751fab cond_timedwait (a304b64, a312b40, ce77def0) + 27
 d2751fe8 pthread_cond_timedwait (a304b64, a312b40, ce77def0) + 21
 d1f5de1c ???????? (a304b64, a312b40, 61a8)
 d1f5dff2 PR_WaitCondVar (a304b60, 61a8) + 66
 08f80379 __1cLnsSSLThreadDRun6M_v_ (a30a810) + 95
 08f7f099 __1cVnsPSMBackgroundThreadOnsThreadRunner6Fpv_v_ (a30a810) + 11
 d1f62ec7 ???????? (a312ba0)
 d2757494 _thr_setup (cea50400) + 52
 d27576f0 _lwp_start (cea50400, 0, 0, 0, 0, 0)
-----------------  lwp# 18 / thread# 18  --------------------
 d2757749 __lwp_park (a3040dc, a312c60, ce32de88) + 19
 d2751962 cond_wait_queue (a3040dc, a312c60, ce32de88, 0) + 3e
 d2751cf7 cond_wait_common (a3040dc, a312c60, ce32de88) + 1e1
 d2751f1c _cond_timedwait (a3040dc, a312c60, ce32df00) + 4a
 d2751fab cond_timedwait (a3040dc, a312c60, ce32df00) + 27
 d2751fe8 pthread_cond_timedwait (a3040dc, a312c60, ce32df00) + 21
 d1f5de1c ???????? (a3040dc, a312c60, 61a8)
 d1f5dff2 PR_WaitCondVar (a3040d8, 61a8) + 66
 08f671c3 __1cYnsCertVerificationThreadDRun6M_v_ (a312c10) + cf
 08f7f099 __1cVnsPSMBackgroundThreadOnsThreadRunner6Fpv_v_ (a312c10) + 11
 d1f62ec7 ???????? (a312cc0)
 d2757494 _thr_setup (cea51000) + 52
 d27576f0 _lwp_start (cea51000, 0, 0, 0, 0, 0)
-----------------  lwp# 20 / thread# 20  --------------------
 d2757749 __lwp_park (a24c7ac, a2655e4, 0) + 19
 d2751962 cond_wait_queue (a24c7ac, a2655e4, 0, 0) + 3e
 d2751e4d _cond_wait (a24c7ac, a2655e4) + 69
 d2751e8b cond_wait (a24c7ac, a2655e4) + 24
 d2751ec5 pthread_cond_wait (a24c7ac, a2655e4) + 1e
 d1f5e005 PR_WaitCondVar (a24c7a8, ffffffff) + 79
 d1f5e2cc PR_Wait  (a2655e0, ffffffff) + 38
 d256aa32 __1cMnsEventQdDueueIGetEvent6MippnLnsIRunnable__i_ (a357120, 1, ce01df34) + e2
 d256c6ff __1cInsThreadQProcessNextEvent6Mipi_I_ (a357100, 1, ce01df60) + bf
 d252344a __1cVNS_ProcessNextEvent_P6FpnJnsIThread_i_i_ (a357100, 1) + 3a
 d256bc4b __1cInsThreadKThreadFunc6Fpv_v_ (a357100) + 117
 d1f62ec7 ???????? (a1fc898)
 d2757494 _thr_setup (cea51800) + 52
 d27576f0 _lwp_start (cea51800, 0, 0, 0, 0, 0)

Comment 1

[timeless]

if you guys are using kmdb or mdb could you /please/ pipe through c++filt?

something like:

::walk thread|::findstack !c++filt

Comment 2

[Aaron Leventhal]

Is this really SunOS only, or does it happen on Linux as well?

Comment 3

[Ginn Chen]

reproduced on Ubuntu

Comment 4

[Nian Liu(n/a in a long time)]

Attachment: patch

null pointer check

Comment 5

[Ginn Chen]

I don't think we should solve this crash this way.

What's the supposed action for the downloaded item?
Why "Open" "Remove" link/button are not created as accessible objects?
(They were there in Firefox 2, I believe.)

Comment 6

[Nian Liu(n/a in a long time)]

(In reply to comment #5)
> I don't think we should solve this crash this way.
> 
> What's the supposed action for the downloaded item?
> Why "Open" "Remove" link/button are not created as accessible objects?
> (They were there in Firefox 2, I believe.)
> 
it's different issue/questions. crash is due to null pointer, isn't it?

Comment 7

[Ginn Chen]

Comment on attachment 246247 [details] [diff] [review]
patch

null check is good, but we should  also fix the action.

Comment 8

[Ginn Chen]

Aaron, do we still need these lines? (nsXULMenuAccessible.cpp, nsXULMenuitemAccessible::DoAction)

268     nsCOMPtr<nsIAccessible> parentAccessible(GetParent());
269     if (parentAccessible) {
270       PRUint32 role;
271       parentAccessible->GetRole(&role);
272       if (role == ROLE_LIST) {
273         nsCOMPtr<nsIAccessible> buttonAccessible;
274         parentAccessible->GetPreviousSibling(getter_AddRefs(buttonAccessible));
275         PRUint32 state;
276         buttonAccessible->GetFinalState(&state);
277         if (state & STATE_PRESSED)
278           buttonAccessible->DoAction(nsXULButtonAccessible::eAction_Click);
279       }
280     }

Comment 9

[Aaron Leventhal]

Ginn, those lines are so that if we're in a combo box, the combo box closes after the option is accessible.

Go ahead and test to see if we still need them. If the combo box closes anyway, then we don't need it. Feel free to do that another better way if you can think of one.

Comment 10

[Aaron Leventhal]

Also, we should add a comment to say what that code is there for, since it's not 100% obvious.

Comment 11

[Ginn Chen]

Aaron, on Linux, combo box doesn't dropdown if I uses at-poke or gok.

And the combox hierarchy is
COMBO BOX
--MENU
----MENU ITEM
----MENU ITEM
The parent should be MENU not LIST.

Comment 12

[Aaron Leventhal]

Ginn, the action #0 has to be performed on the combo box button. I guess that's not supposed to be exposed under ATK/AT-SPI. 

Let's fix the crash first and then reconstruct the combobox accessibles to make the combo box get exposed for ATK/AT-SPI as it is supposed to be. However, I'd like to try to have the nsIAccessible implementation be the superset of objects, and trim out any unnecessary objects in mozilla/accessible/src/atk. If necessary we can create a new nsIAccessible::ROLE_COMBOBOX_LIST and nsIAccessible::ROLE_COMBOBOX_OPTION which get mapped to LIST/MENU roles as appropriate on each platform.

Comment 13

[Ginn Chen]

Aaron, I checked the current hierarchy of combobox on both Linux and Windows.
They're almost same.

For XUL combobox, it's like 
COMBO BOX (no action)
--MENU (no action)
----MENU ITEM (action "check")
----MENU ITEM (action "check")

For HTML combobox, it's like
COMBO BOX (no action)
--ENTRY (action "Activate")
--BUTTON ( action "Open")
--LIST (no action)
----LIST ITEM (action "Select")
----LIST ITEM (action "Select")

For HTML combobox, nsHTMLSelectOptionAccessible::DoAction will take care.
I think we can remove these line from nsXULMenuAccessible.cpp until we reconstruct XUL combobox accessible.

Comment 14

[Ginn Chen]

Attachment: patch

Comment 15

[Aaron Leventhal]

Actually I like the previous patch better because it does not regress the Windows behavior. I would just add a comment and an additional check to make sure the grandparent is a combo box.

Comment 16

[Aaron Leventhal]

Ginn, will there be a time when we can talk about this bug on IRC? It's hard to do it through bugzilla.

You're right, our combo boxes in XUL are inconsistent with what we have in HTML. There's a bug open on that. I guess there is a small chance we can just make them all like the ones in XUL, for consistency between HTML/XUL and Linux/Win.

Comment 17

[Aaron Leventhal]

I'd prefer to use the first patch for now. We'll deal with the whole combo box picture when we can.

Comment 18

[Ginn Chen]

Because the XUL combo box is not like HTML select for now.
These lines can never be run.
I prefer remove or comment out these lines, till we solve XUL combo box.
It won't regress any Windows behaviour, since there's no chance to run it.

Comment 19

[Aaron Leventhal]

Comment on attachment 246882 [details] [diff] [review]
patch

Ah ok. I see.