403174 - [10.5] Sheets come out above toolbar instead of below it

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Samuel Sidler (old account; do not CC)]

Native apps on 10.5 have sheets which come out below the toolbar. Firefox still has the behavior seen in 10.4, with the sheets coming out above the toolbar.

Comment 1

[Colin Barrett [:cbarrett]]

From the Appkit Release Notes on Sheets for 10.5:
> Due to compatibility concerns, the additional content border thickness is only
> taken into account for positioning the sheet if it has been set explicitly. 
> That is, if the window returns YES from [self
> autorecalculatesContentBorderThicknessForEdge:NSMaxYEdge], the auto-calculated
> content border thickness is not included when positioning the sheet.

So what we would need to do is use [window setContentBorderThickness:height_of_toolbar forEdge:NSMaxYEdge].

No idea how you would communicate that information to widget from XUL-land, and the new sheets only existing in content seems like it's a bad idea for security (safari sort of mitigates this by having it cover the tab bar, but if you don't have any tabs open...)

Comment 3

[Markus Stange [:mstange]]

Attachment: fix per comment #1

Bug 439354 makes this possible.

Comment 4

[Markus Stange [:mstange]]

Attachment: Correctly attached sheet in Thunderbird with fix

Comment 5

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/84bbd1f88fac

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Sheet from above the toolbar

Markus, please correct me when I'm wrong but it still doesn't work for me with the latest nightly build. Please see both attachments which show that the sheets are places above or below the toolbar.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Sheet below the toolbar

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Both can be seen with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080921021112 Minefield/3.1b1pre ID:20080921021112

Comment 9

[Markus Stange [:mstange]]

Funny. I'll back out when I have time to.

Comment 10

[Markus Stange [:mstange]]

Backed out.

Comment 11

[Markus Stange [:mstange]]

OK, the difference in behaviour from the nightly build and my local build was caused by the fact that I wasn't building with the 10.4 SDK. When I build with the 10.4 SDK, I see the bug.

I'll go debug a little.

Comment 12

[Markus Stange [:mstange]]

It looks like setContentBorderThickness is just buggy when building with the 10.4 SDK. I don't know what to do about this - Josh, do you have any ideas?

Comment 13

[Alex]

When this was in the nightly, I was getting a crash on start, since it was backed out the latest nightly loads fine. It's confusing since what the crash reporter is reporting, isn't what's in the patch (NSMinXEdge vs. NSMaxYEdge)

"Obj-C Exception data: NSInvalidArgumentException: setContentBorderThickness:forEdge: may not be called with NSMinXEdge"

I'm on Leopard PPC, could something be different in the compiling process? Here's a report on the crash server, http://crash-stats.mozilla.com/report/index/bcb1330c-886d-11dd-b393-001cc4e2bf68

Comment 14

[Markus Stange [:mstange]]

There's probably nothing we can do here as long as we're building with the 10.4 SDK.
Somebody should try to create a reduced testcase and file a bug at Apple.

Comment 16

[Markus Stange [:mstange]]

Attachment: v2

Now that we're using the 10.5 SDK this should work glitch-free.

Comment 17

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/90c81f1fb23d

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There is a gap now between the sheet and the collapsed toolbar. See bug 528830.