Closed Bug 431429 Opened 16 years ago Closed 16 years ago

App focus wierdness with sheets

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: smichaud, Assigned: smichaud)

Details

(Keywords: regression)

Attachments

(1 file)

I've found a serious UI problem that should (I think) block the 1.9
release.

1) Run Minefield and open a sheet over the single browser window --
   say by entering the following in its location bar:

   javascript:alert('whatever');

2) Click on the desktop to take the application focus away from
   Minefield.

3) Click on the browser window's content area -- i.e. not on its title
   bar or on the open sheet.

   Notice that the menu doesn't change back to the Minefield menu.
   But the sheet has (somehow) received the keyboard focus -- if you
   press Esc the sheet will close.

   However, once the sheet is closed, clicking on the browser window
   (anywhere on it) doesn't restore the Minefield menu.	 Though	it's
   possible to enter text in any of that window's text-input fields
   (after you've clicked on them).

   In order to restore the Minefield menu, you must click on the
   desktop (or another app) and back (again) on the Minefield browser
   window.

This is a recent regression.  It doesn't happen with the 2008-03-23-04
Minefield nightly, but does happen with the 2008-03-24-04 nightly.
The most likely cause is the patch for bug 395465.

I'll be	working on this.
Flags: blocking1.9?
Assignee: joshmoz → smichaud
Keywords: regression
FYI, I believe the sheet receiving focus (first part of step 3), despite clicking elsewhere, is normal behaviour for sheets on OS X; sheets are window-modal dialogs and clicking anywhere in the window they're associated with will put focus in the sheet.
Attached patch FixSplinter Review
This bug has an easy fix.

The patch for bug 395465 introduced special treatment for the windows
above both kinds of modal window -- 1) free-standing app-modal windows
and 2) window-modal sheets attached to a window.

But the special treatment for sheets became moot (and unnecessary)
when another part of that patch was dropped before it was landed (the
issue is now covered by bug 424424).

And the special treatment for sheets introduced this bug -- by eating
all mouse events that happen above the part of a window not covered by
its sheet (for as long as the sheet is displayed).

So I've removed it.

We'll have to revisit this when we tackle bug 424424.  But fortunately
bug 424424 isn't urgent, and has been put off for a point release.

A tryserver build will follow shortly.

In a few hours I'll post another tryserver build that combines this
patch with patches for bug 406730 and bug 428071.
Attachment #318615 - Flags: review?(joshmoz)
Attachment #318615 - Flags: review?(joshmoz) → review+
> In a few hours I'll post another tryserver build that combines this
> patch with patches for bug 406730 and bug 428071.

I've made my combo tryserver build (with a combo patch) available at
bug 406730 comment #97.
Attachment #318615 - Flags: superreview?(roc)
Attachment #318615 - Flags: superreview?(roc) → superreview+
I'm assuming that this fix is actually independent of the patches in bug 406730 and 428071, which we aren't going to land for 1.9. Speak up if it does depend on them.
> I'm assuming that this fix is actually independent of the patches in
> bug 406730 and 428071

Yes, it's completely independent.
Comment on attachment 318615 [details] [diff] [review]
Fix

This is no longer needed by the patches for bug 406730 and bug 428071
(which aren't going to land in the near future).

But it still fixes a significant bug, at very little risk.

Furthermore, this bug may be worse than it first appears -- who knows
what else is wrong under the hood when a browser window gets the
keyboard focus without the browser's menu becoming active.
Attachment #318615 - Flags: approval1.9?
Comment on attachment 318615 [details] [diff] [review]
Fix

a1.9+=damons
Attachment #318615 - Flags: approval1.9? → approval1.9+
Landed on trunk:

Checking in widget/src/cocoa/nsCocoaWindow.mm;
/cvsroot/mozilla/widget/src/cocoa/nsCocoaWindow.mm,v  <--  nsCocoaWindow.mm
new revision: 1.144; previous revision: 1.143
done
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: