393250 - titlebar=no windows do not receive keyboard events

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Windows without titlebars are broken in Cocoa widgets. Mouse events seem to work just fine but keyboard events are not focused correctly. I have a small XULRunner app as a testcase hosted here:

http://dingo.songbirdnest.com/~ben/mozilla/titlebartest.zip

Unfortunately I can't put it in bugzilla as it is about 10mb...

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Test XUL page

Ok, Håkan reminded me that I could do this easier... Thanks!

So to test save this file on your hard disk somewhere. Then type this into the error console:

  window.open('file:////Users/ben/Desktop/test.xul', '', 'chrome,centerscreen,titlebar=no');

That should do it (as long as you substitute the file path appropriately).

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Oh, and the testcase works fine on the 1.8 branch.

-> regression

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

Here's a fix for this bug.

Turns out that Cocoa special-cases windows without titlebars -- by
default they can't become either the "key window" or the "main
window".  (If a window can't become "key", it can't accept keyboard
input.)

I had to play some tricks to work around this.

Even after this fix, though, there's still some other wierdness.  I
can't figure out whether or not it's by design:

After the fix, a window created using the procedure in comment #1 has
only a very truncated main menu -- it just includes the Apple and
"Minefield" top-level menu items.  (It's not modal, though.)

And even before the fix, this is true of a window created using the
following command in the Error Console:

  window.open('file:////Users/ben/Desktop/test.xul', '',
    'chrome,centerscreen');

This looks intentional ... but is it?

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Regardless of the verdict on the "truncated main menu" issue (whether
it's a bug or a feature), that's a separate matter, unrelated to this
bug.  It shouldn't block the landing of this bug's patch.

If we decide the "truncated main menu" is a bug, I'll open a new bug
on it.

Comment 5

[Josh Aas]

Comment on attachment 284984 [details] [diff] [review]
Fix

Not going to be this easy unfortunately. This causes key window handlers to kick in and whatnot, leading to different event routing and menu bars.

One thing this breaks is the Places toolbar item. Open that and try to select an item from "Most Visited Pages". It doesn't work and the submenu kicks up a new menu bar.

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Just to chime in, we use a titlebar=no window as our main application, so we would consider the truncated menu as a bug :)

Comment 7

[Steven Michaud [:smichaud] (Retired)]

> Just to chime in, we use a titlebar=no window as our main
> application, so we would consider the truncated menu as a bug :)

So it sounds like the truncated menu problem actually cripples your
"main application" (which I assume is Songbird).

(I'm trying to figure out how serious the truncated menu problem is.)

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #7)
> So it sounds like the truncated menu problem actually cripples your
> "main application" (which I assume is Songbird).

Correct.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix rev1 (fix dumb mistake)

I made a dumb mistake in my previous patch -- I forgot that popup
windows are also borderless (have no titlebar).  So I inadvertently
changed every popup window from class PopupWindow to class
BorderlessWindow ... which would have messed up all popup windows :-(

Thanks, Josh, for noticing that there was something wrong :-)

Anyway, _this_ patch should do the trick.

(It doesn't (of course) fix the truncated menu problem.  I'll open a
new bug on that.)

Comment 10

[Steven Michaud [:smichaud] (Retired)]

> (It doesn't (of course) fix the truncated menu problem.  I'll open a
> new bug on that.)

I've opened bug 400084 and assigned it to myself.

Comment 11

[Steven Michaud [:smichaud] (Retired)]

I'm going to have to post another revision -- I noticed that a window
created using the following still doesn't become main (or get keyboard
events):

  window.open('file:////Users/ben/Desktop/test.xul', '',
    'titlebar=no');

Sigh :-(

Comment 12

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 285162 [details] [diff] [review]
Fix rev1 (fix dumb mistake)

> I'm going to have to post another revision -- I noticed that a
> window created using the following still doesn't become main (or get
> keyboard events):
>
>  window.open('file:////Users/ben/Desktop/test.xul', '',
>     'titlebar=no');

Turns out it was my test that was wrong.

As far as I can tell, my rev1 patch (attachment 285162 [details] [diff] [review]) works and has
no side effects.

Comment 13

[Steven Michaud [:smichaud] (Retired)]

I will land this when the tree reopens.

Comment 14

[Steven Michaud [:smichaud] (Retired)]

Landed on trunk.

Comment 15

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Yay, thanks!

Comment 16

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007102404 Minefield/3.0a9pre. I used bent's wonderful test case to verify.