290862 - Keybindings not disabled in parent of a modal dialog using gtk2 build

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Rob Adams]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.6) Gecko/20050405 Firefox/1.0 (Ubuntu package 1.0.2)
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.6) Gecko/20050405 Firefox/1.0 (Ubuntu package 1.0.2)

WHen a modal dialog is open, the parent should ignore all input, including
keybindings.  Keybindings are currently not ignored

Reproducible: Always

Steps to Reproduce:
1. Open modal dialog
2. Use keybinding in parent, such as Ctrl-W to close the window
3. Bad things now happen

Actual Results:  
Parent window closes

Expected Results:  
Parent window ignores keybinding

gtk2 build, linux, trunk, xulrunner

Comment 1

[Boris Zbarsky [:bzbarsky]]

Confirming.  "Bad things" here means a crash.  This is specific to GTK2 builds,
looks like.

bryner, blizzard, any idea what could be up here or who has time to deal with
this?  This is something we really want to fix, imo.

Comment 2

[Asa Dotzler [:asa]]

doesn't look like anything's happening here for beta2

Comment 3

[Benjamin Smedberg]

I don't want to ship 1.1 with this.

Comment 4

[Benjamin Smedberg]

Chris, can you take a look at this?

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

if this is limited to the gtk2 filepicker (I can't tell, the bug has no steps to
reproduce), then this may be identical to the (now fixed) bug 301004.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

> the bug has no steps to reproduce

(no detailed enough ones, that is)

Comment 7

[Rob Adams]

Any modal XUL dialog where the parent window has keybindings configured.  The
firefox preferences dialog is one example.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Steps to reproduce:

1)  Click the "URL" link in this bug.
2)  Use keybindings in the window while the modal alert is up.

Comment 9

[Asa Dotzler [:asa]]

Where does it crash? I'd like to see if this is in our topcrash data or not.

Comment 10

[Boris Zbarsky [:bzbarsky]]

It's not crashing with current trunk in a simple testcase, just asserting a lot.

That said, unless we're sure all of our chrome deals with this situation (which
it's not expecting to happen), we should really fix this.  I'm assuming we don't
care about content script being broken by this, of course.

Comment 11

[Asa Dotzler [:asa]]

not a topcrash. we'd consider a reviewed patch if it was low risk.

Comment 12

[Matthew Gregan [:kinetik]]

I can reproduce this on trunk, but it doesn't crash for me.  Also, this bug looks identical to bug 280895.

We are using gtk_grab_add/remove in nsWindow::SetModal to block events to the parent window.  This affects GTK widget behaviour but not the window manager behaviour.  The grab causes events to the parent widget to be redirected to the (modal) child widget, but our code dispatches key press events to gFocusWindow.  gFocusWindow tracks the focused window via focus events, so what happens when a modal dialog is up and the focus changes to the parent window is that we receive events for the modal child widget (due to grab redirection) and redispatch them to the parent window.

Comment 13

[Matthew Gregan [:kinetik]]

Attachment: patch

Two pronged approach to fix this.  Use gtk_window_set_modal instead of gtk_grab_add.  set_modal sets the WM modality hint as well as adding a grab.  For WMs that honour the modality hint, this stops the parent window from receiving focus.

For WMs that do not honour the modality hint, block focus changes (i.e. gFocusWindow changes) internally by setting mContainerBlockFocus on the parent of modal dialogs.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

I'd like Karl to check this too, since my GTK knowledge is sketchy, but it looks good.

Comment 15

[Boris Zbarsky [:bzbarsky]]

gtk_window_set_modal doesn't affect the event loop in that, e.g. network loads in the window that has a modal child will still happen, right?

Comment 16

[Matthew Gregan [:kinetik]]

Correct--gtk_window_set_modal is a wrapper around calls to gdk_window_set_modal_hint + gtk_grab_add/remove, so the behaviour of the event loop is unaffected.

Comment 17

[Karl Tomlinson (:karlt)]

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

Looks good to me.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is a rather bad bug. Now that we have a patch, I see no need to not treat it as a blocker.

Comment 20

[Damon Sicore (:damons)]

Did this get checked in and fixed?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No, I'm going to check it now.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in.

Comment 23

[u279076]

Using Firefox 3.0b5 and the following STR, I can reproduce this bug...

1. Open firefox
2. Open a new tab
3. Open Help>About dialog or Tools>Addons dialog
4. Click on content area of the new tab
5. Press CTRL+W

RESULT:
The tab is closed.

EXPECTED RESULT:
Shortcut should do nothing.

Perhaps I am wrong in what this bug was about?

Comment 24

[Matthew Gregan [:kinetik]]

The about and addons dialogs are not modal, you're free to interact with the parent browser window while they are open.

Comment 25

[u279076]

Kindly provide me with some dialogs that are modal than.

Thank you.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Please see comment 8.