Sheet dialog support is implemented incorrectly for MacOSX. You can see this best by opening Tools->Extensions in Firefox, and clicking on the 'about' button for one of the extensions. This will open a floating window in the top left corner with no titlebar, no where near the Extensions window. Same thing happens for the extensions options/preferences dialog. The reason for this is that sheet support is implemented as a bit in the nsBorderStyle member of the nsWidgetData structure. However, the eBorderStyle_default enum is defined as '-1', which becomes '0xffff' (all bits set), meaning we can't tell whether the eBorderStyle_sheet bit has been set at all.
The issue is that sheet support was implemented in the wrong place. It is not a border style; sheets are actually a special type of dialog. So in this patch I've defined a new nsWindowType to handle sheets.
Assignee: jag → jhpedemonte
Status: NEW → ASSIGNED
Is this the reason that I see almost all pulldown windows appearing in the left-hand corner, both in todays Seamonkey and Firefox builds (2004083108, on Mac OS X 10.2.3) ? I haven't seen this in yesterdays builds (2004083008). Erasing the XUL.mfasl file didn't help. I see it with : - location pulldown - search engine pulldown (in FF) - right click pulldown on toolbar (icon-pulldowns are ok) - all right-click popups in the browser area - most title-popups Once in a while I see pulldowns that are in the correct place, or that jump back to the correct place (esp when closing the pulldown again), but that's not repeatable. Most pulldowns that are attached to buttons (in the mail-interface f.e.) work ok, except for the location pulldown and the search engine pulldown. A major regression caused by bug 223545 (which I love btw) ?
Jo, please post a comment in that bug, and make sure to mention your OS version. This bug should be unrelated to what you are seeing.
Attachment #157502 - Flags: review?(peterv)
(In reply to comment #2) > Is this the reason that I see almost all pulldown windows appearing in the > left-hand corner, both in todays Seamonkey and Firefox builds (2004083108, on > Mac OS X 10.2.3) ? I haven't seen this in yesterdays builds (2004083008). > Erasing the XUL.mfasl file didn't help. This is fixed now (bug bug 257647)
QA Contact: jrgmorrison → bugs.mano
*** Bug 259343 has been marked as a duplicate of this bug. ***
Our sheet impl' is really problematic, your patch does impore it, but the main problem is still there: no way to create modal-but-non-sheet windows. This is a very bad user-exp' when it comes to modal-opened-by-modal behavior, that's one reason that not every document-modal on OS X is a sheet. So what this one changes/adds: 1) "modal, centerscreen" is a normal document-modal window, not a sheet --This adds an option for window.open() callers to avoid sheet behavior. Cases that are fixed without anymore changes are wizards and firefox/tb about box. 2) Disables autosheet of dialogs (that's because not every dialog is per-current-document) 3) prompts are just "chrome, modal" on OS X (aka sheets) 4) cleanup of nsAppShellService::CalculateWindowZLevel
Attachment #163009 - Attachment description: proposed addition → proposed addition (diff -u)
mano: Do you have any specific examples of dialogs that your additions change? Like a dialog that is now (or no longer) a sheet?
(There ar no new sheets.) real life examples for no-sheets (after patching): About Dialog Import Wizard Profile Migrator Some windows in the address book There are some cases that we may want to "resheet" like the download-properties dialog, but lets get this one in, so we will be able to contorl what is "sheeted". btw, 1.0mac was dropped, nominating for 1.0.
Flags: blocking-aviary1.0mac? → blocking-aviary1.0?
Comment on attachment 163009 [details] [diff] [review] proposed addition (diff -u) we're really short of time for 1.0 (as mentioned no separate 1.0mac anymore).
this is pretty risky at this stage in the game. how much testing has it had?
(In reply to comment #11) > this is pretty risky at this stage in the game. how much testing has it had? Well, pedemonte's patch is applied on my build for some time and from what i can say, it hasn't regressed anything. I haven't played enough with the additions from comment 9, but i am not sure they're that risky, we don't have many modal,centerscreen windows in Firefox (We do have in tbird, but there is much more time until the next release). After this one is landed we're going to "desheet" some windows, especially those that has sub-modals (Firefox's prefs window for example and some wizards). Anyway, the orginal issue (which is fixed by the patch) is very visible in FF; extensions' options panes and some pref windows (e.g. js options) are opened as modal in screen's top left corner.
Simon, I don't think the first patch is that risky at all. It only makes it so the sheet flag isn't obscured. As Asaf says, this specifically fixes the issues in the Firefox Extensions dialog, when selecting Preferences or About. I'm not sure about Asaf's additions. I think that one might be too risky; we might want to wait until after 1.0. I'll have to talk to him about it.
Comment on attachment 157502 [details] [diff] [review] patch (diff -w) Looks ok. I do think we should go for a distinction between application-level dialogs and window-level dialogs in the future (and use that concept in the enum's names and let other platforms treat them as equal). I don't think CHROME_OPENAS_DIALOG | CHROME_MODAL is completely correct in that sense. But this is fine for now I think.
Attachment #157502 - Flags: review?(peterv) → review+
peterv, I agree completely. That's on my plate, just haven't gotten around to it yet.
Attachment #157502 - Flags: superreview?(sfraser)
Attachment #157502 - Flags: superreview?(sfraser) → superreview+
Comment on attachment 163009 [details] [diff] [review] proposed addition (diff -u) obselting this one, for now.
Whiteboard: [have patch]
Whiteboard: [have patch] → [have patch] - needs approval ben
I applied the patch to a fresh Aviary build and it looked to me like application-modal dialogs are being used instead of sheets. This works for Wizards, Preferences and the About dialog, but I would hope that if the patch is considered for 1.0 that it include the use of sheets where appropriate. Should be sheets (Firefox): Alert/Auth Dialog (Common Dialog) Cert Warning Dialog Add Bookmark dialog Software Install Allowed Sites (triggered from page info bar) Software Install Popup Blocker Allowed Sites (triggered from page info bar) Should not be sheets: Preferences About Wizards
(In reply to comment #17) what patch did you applied? the first one (which is the nominated one) doesn't change the behavior you mentioned (while the last one does).
I applied the first patch (sheet.diff) ... perhaps I applied it incorrectly (patch -p0 < sheet.diff) but I didn't get errors.
(In reply to comment #19) Well, everyting is still "sheeted", here.
We can't take this kind of thing now. We're shipping on Tuesday.
Flags: blocking-aviary1.0? → blocking-aviary1.0-
Comment on attachment 157502 [details] [diff] [review] patch (diff -w) sigh
Checked in first patch to trunk. I'm going to mark this as fixed, since the original problem this bug was opened for has been fixed. I'll open another bug to handle Asaf's concerns.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Whiteboard: [have patch] - needs approval ben
Attachment #163010 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.