266769 - parent window isn't movable / draggable when sheet is open

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Mac, defect)

Description

[David Catmull]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20041026 Firefox/1.0RC1
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20041026 Firefox/1.0RC1

When a sheet is open, such as an error alert, the about box, or the preferences
window, the parent window is not draggable.

Reproducible: Always
Steps to Reproduce:
1. Have a browser window open
2. Select About Mozilla Firefox or Preferences from the application menu
3. Try to drag the browser window

Actual Results:  
It beeps.

Expected Results:  
The window should be draggable; that's a basic part of how sheets are supposed
to work.

Comment 1

[Asaf Romano (gone)]

Looks as we don't have an older report, confirmed.

Comment 2

[Frankie]

*** Bug 276425 has been marked as a duplicate of this bug. ***

Comment 3

[Ari]

Attachment: potential patch

This seems to fix it. I haven't noticed any ill effects. Does anyone know the
justification for the check that I removed?

Comment 4

[Simon Fraser [no longer active]]

CVS blame:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/mac/nsMacMessagePump.cpp

says that change was made to fix bug 126786.

Comment 5

[Ari]

OK, now I think I know what's going on:

The check was added in bug 126786, but was not meant to apply to OS X. In fact, the original check 
was: 

+        if (nsToolkit::OnMacOSX() ||
+            !topWidget || NS_FAILED(topWidget->IsEnabled(&enabled)) || enabled ||
+            (anEvent.modifiers & cmdKey)) {

See comments 9 and 10 in bug 126786.

But then, the patch for bug 223545 removed the "nsToolkit::OnMacOSX()". Since that bug doesn't seem 
to have anything to do with dragging, but did replace nsToolkit::OnMacOSX() with nsToolkit::
OSXVersion(), I think this is an accidental regression. Since OS9 is dead, we can probably just remove 
the whole check.

Comment 6

[Simon Fraser [no longer active]]

This is a serious bug that is super annoying, and can cause FF to get into a
locked state if window layering changes while a sheet is up and FF is in the
background.

Comment 7

[Mark Mentovai]

Comment on attachment 170620 [details] [diff] [review]
potential patch

This is the right thing to do.	r=me, as long as the indentation is fixed to
reflect that ::DragWindow and friends have moved up a level.

Here's the regression, where it went wrong:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&r
oot=/cvsroot&subdir=mozilla/widget/src/mac&command=DIFF_FRAMESET&file=nsMacMess
agePump.cpp&rev2=1.143&rev1=1.142

Comment 8

[Simon Fraser [no longer active]]

Comment on attachment 170620 [details] [diff] [review]
potential patch

Thanks for the archaeology.

Comment 9

[Mark Mentovai]

Fixed on the trunk.  This is a regression, though it regressed before 1.7/aviary
1.0 - let's get this for the branch too if we can.  Low risk.

Comment 10

[Josh Aas]

I recommend taking this on the branch.

Comment 11

[Asaf Romano (gone)]

Checking in nsMacMessagePump.cpp;
/cvsroot/mozilla/widget/src/mac/nsMacMessagePump.cpp,v  <--  nsMacMessagePump.cpp
new revision: 1.148.8.1; previous revision: 1.148
done

Comment 12

[Scott MacGregor]

belatedly setting the blocking 1.8b5 flag since we took this fix on the branch.