222488 - dialogs with default style flag do not have a title bar and are positioned oddly

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Mac, defect)

Description

[Bruce Davidson]

When "Find nodes" is selected in DOM inspector it should open as a Mac OS X
sheet (if its supposed to be modal) underneath the DOM inspector window's title
bar. Alternatively it should open as a non-modal dialogue box with its own title
bar.

Currently it opens as a sheet but at the top left of the DOM inspector window,
on top of the DOM inspector's title bar. Sheets should be centered relative to
their parent window and drop down from under the title bar.

Comment 1

[Bruce Davidson]

Attachment: Screenshot

Screenshot of "find nodes" being neither sheet nor dialogue.

Comment 2

[timeless]

reporter: the find dialog, like most find dialogs is not supposed to be modal.

it's opened 

this is inspector:
var win = openDialog("chrome://inspector/content/viewers/dom/findDialog.xul",
"_blank", "chrome,dependent", this.mFindType, this.mFindDir, this.mFindParams);

this is navigator:
window.findDialog = window.openDialog("chrome://global/content/finddialog.xul",
"_blank", "chrome, resizable=no,dependent=yes", findInstData);

this is navigator (c++):
rv = OpenDialog(NS_LITERAL_STRING("chrome://global/content/finddialog.xul"),
                NS_LITERAL_STRING("_blank"),
                NS_LITERAL_STRING("chrome, resizable=no, dependent=yes"),

They all set chrome and dependent, none of them set modal.

Comment 3

[[e-mail]]

Since I don't know anything about sheets, are they movable? Both Navigator and
DOM Inspector find dialogs expect to be movable and remember their position.

Comment 4

[Simon Fraser [no longer active]]

The sheet implementation in Mozilla is an abhorrence, and should be turned off.

Comment 5

[Mike Pinkerton (not reading bugmail)]

it needs tweaking, but isn't an abhorrance. i much prefer its behavior than
popping up dialogs on the wrong monitor (which is what would happen if we turned
it off).

Comment 6

[Josh Aas]

Attachment: fix v1.0

This has nothing to do with sheets. What you're seeing is just a window without a titlebar, and that alone does not a sheet make. We're not requesting that the dialog be modal, and in widget code we're getting a request for a window type of eWindowType_dialog when creating the DOM Inspector find window.

The problem here is that the style flags on the dialog request are "default", and at one point somebody decided that "default" is the same as "none", which is pretty bogus. The reason we don't have this problem if you specify resizable=no is that in that case instead of only having the style mask bit for default set, we get the default bit plus some other bits like title and close flags. That causes the switch case for (borderstyle == default) to fail and we go into the default processing which throws a title bar on the window.

The fix here is to give the window a reasonable window class when the default flag is the only flag set.

Comment 7

[Mark Mentovai]

Comment on attachment 228731 [details] [diff] [review]
fix v1.0

>               break;
>+ 
>+            case eBorderStyle_default:

There's a space on that blank line, get rid of it.

Comment 8

[Josh Aas]

This is a pretty bad bug with a pretty safe fix.

Comment 9

[Josh Aas]

landed on trunk

Comment 10

[Josh Aas]

I checked in a followup to this changing:

-              windowClass = kMovableModalWindowClass;
+              windowClass = kDocumentWindowClass;

We don't want to force modality on the dialog.

Comment 11

[Josh Aas]

Attachment: 1.8.1 branch fix v1.0

fix for the 1.8 branch plus a comment change

Comment 12

[Mark Mentovai]

Comment on attachment 228814 [details] [diff] [review]
1.8.1 branch fix v1.0

That's better.

Comment 13

[Josh Aas]

landed on 1.8.1 branch