396559 - window.blur doesn't work

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Stuart Morgan]

window.blur() should send the window behind other windows (and does in Firefox), but in Camino it doesn't do anything.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Since this is another web-compat issue, nominating for 1.5.2, but it may be unreasonable to expect a patch.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Not making 1.5.2, but for web-compat, we do want this whenever we get a patch.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Ditto comment 2 for 1.5.4.

Comment 4

[Nick Kreeger]

Attachment: Patch V1

Not trying to step on your shoes here smorgan, but I remembered that this function was blank in CHBrowserListener...

Technically - this should work, however the call to |[window orderBack:nil]| hides the BrowserWindow... Not sure why, but I'll continue to look into the actual cause of this.

Comment 5

[Nick Kreeger]

+  if (window)
+      [window orderBack:nil];

Sorry bout the tab..

Comment 6

[Nick Kreeger]

Attachment: Patch V1a

This is a bit more of a hack - but it might be one of our options outside of the normal NSWindow API's. After digging around in MainController, I'm seeing lots of comments about BrowserWindow's not always working the right way with the  [NSApp mainWindow] calls. I'm assuming this might be related. 

If we want to pursue down this path, we could add another method to MainController instead of exposing |browserWindows:| to callers (although it seems to fit the task at hand since we don't want to blur the window if there aren't any other windows around).

I'm targetting smorgan to take a look at this, I'm sure he will have some better insight into this than I ;-)

Comment 7

[Stuart Morgan]

Comment on attachment 292026 [details] [diff] [review]
Patch V1a

orderBack: wouldn't be enough even if it worked, since it's documented to leave key and main window status unchanged, which isn't what we want, so we'll need to do something with the browser window list regardless.

Adding a dependency on MainController to CHBrowserListener isn't even remotely an option though; no CH* class can depend on a higher-level Camino class. You'll need to either pass this out through the CHBrowserListener protocol and up from BrowserWrapper, or through the informal NSWindow(BrowserWindow) protocol in CHBrowserListener to a new method that's added to BrowserWindow (e.g., resignKeyAndOrderBack). The former is probably better.

Note that either way you need to put in the same kind of protection against background tabs triggering this that SetFocus() has.

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Minusing for 1.5.5; if there's a patch, we'll consider for 1.5.6 if it happens.

Comment 9

[Nick Kreeger]

Attachment: Patch V2

Here is another go-round at this. It adds a new method to the |BrowserWindow| class that takes care of the shuffling of windows.

This seemed a bit more appropriate considering this is the action that |SetFocus()| takes. (rather than routing through a new protocol method).

Comment 10

[Stuart Morgan]

Comment on attachment 302848 [details] [diff] [review]
Patch V2

Architecturally it looks good, but the window reordering is wrong. This makes the rear-most window come to the front and leaves the old window second, rather than sending the front window to the back, making the second window front.

You want:
    [(NSWindow*)[browserWindows objectAtIndex:1] makeKeyAndOrderFront:nil];
    [self orderWindow:NSWindowBelow relativeTo:[[browserWindows lastObject] windowNumber]];

Comment 11

[Stuart Morgan]

Although, the more I think about it, the more I'm convinced that this should be part of CHBrowserContainer. Unlike suppressMakeKeyFront, which is optional and helps us work around issues at the Camino level, your new method is critical to the basic implementation of a web compatibility method; it's much more like closeBrowserWindow, which is a CHBrowserContainer method.

Comment 12

[Stuart Morgan]

Not a blocker for 1.6, but certainly a nice to have.

Nick, are you planning on tweaking this in the near future, or should someone else take this over?

Comment 13

[Stuart Morgan]

Attachment: v3

Taking back so that we can make sure this gets into 1.6. Thanks for doing all the leg work on this, Nick!

Comment 14

[Mike Pinkerton (not reading bugmail)]

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

sr=pink

+- (void)resignKeyAndOrderBack

can you add a function-comment on why this is needed?

Comment 15

[Stuart Morgan]

Landed on trunk and MOZILLA_1_8_BRANCH, with a comment explaining that resignKeyAndOrderBack is for window.blur support.

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Camino 1.5.6 is not happening.