515446 - Creating popup windows incorrectly interprets aNativeParent as an NSWindow when ui.use_native_popup_windows is turned on

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P1)

Description

[Benjamin Stover (:stechz)]

Attachment: Output of running

Go to www.tv5.org.  Soon, Fennec will seem to hang.  Loading animation still spins, but I can't pan or click the awesome bar.

Reproducible: always
Platform: Mac xulrunner debug

Comment 1

[Stuart Parmenter]

this looks to be a widget bug, related to the:
2009-09-09 12:05:23.315 fennec[30935:903] -[ChildView orderOut:]: unrecognized selector sent to instance 0x1d18ad50
exception

Comment 2

[Josh Aas]

orderOut: is a selector on a top-level window (NSWindow), it does not exist in child views (NSView). We should never be calling orderOut: on a child view, blocking1.9.2+.

Comment 3

[Markus Stange [:mstange]]

This is a regression from bug 420491, part 2. That patch changed how we handle the combination of aNativeWindow && UseNativePopupWindows().

Comment 4

[Markus Stange [:mstange]]

Attachment: v1

When <select> popup widgets are created, the parent child view's native widget is passed to aNativeWidget. We were misinterpreting it as an NSWindow even though it was an NSView.
Fennec sets ui.use_native_popup_windows to true so that we don't create a widget.

Comment 5

[Markus Stange [:mstange]]

Comment on attachment 401160 [details] [diff] [review]
v1

Canceling review here - Josh and I agreed that window creation is some convoluted mess that definitely needs cleanup, so I'll do that instead.

Comment 6

[Josh Aas]

Marking this as a P1 beta blocker. The code in question has higher than normal potential for regressions.

Comment 7

[Markus Stange [:mstange]]

That means it blocks Beta and I need to get it in yesterday or something?

Comment 8

[Josh Aas]

I think beta bugs need to be in by next week.

Comment 9

[Markus Stange [:mstange]]

Attachment: v2

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 402261 [details] [diff] [review]
v2

Need to rename StandardWindowCreate on Windows

Comment 11

[Markus Stange [:mstange]]

also aParent -> aNativeParent in the Qt and Windows nsWindow.h, and aNativeWindow -> aNativeParent in nsCocoaWindow

Comment 12

[Markus Stange [:mstange]]

Attachment: v3

Comment 13

[Peter Weilbacher]

Markus, may I ask you to change the formatting in the OS/2 files to remove the extra spaces after opening brackets, i.e. "( " -> "(" and treat ifs properly, like "if( " -> "if ("?
I know you are just keeping to the old style but we are trying to gradually remove that from OS/2's nsWindow. (It's good enough if this is done on checkin.)

Comment 14

[Markus Stange [:mstange]]

Sure, I'll do that.

Comment 15

[Josh Aas]

Comment on attachment 402280 [details] [diff] [review]
v3

+  // Applications that use native popups don't us to create popup windows.

I think you forgot the word "want".

Comment 16

[Markus Stange [:mstange]]

Indeed.

Comment 17

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/920142c5558f

(In reply to comment #13)
I also changed the 3-space indent to 2-space indent.

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

This needs to land on 1.9.2 before we can ship beta.

Comment 19

[Markus Stange [:mstange]]

Oops, sorry, forgot to update the bug... this already landed:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4a36e4cba6d6