window.blur doesn't work



Camino Graveyard
10 years ago
10 years ago


(Reporter: Stuart Morgan, Assigned: Stuart Morgan)






(1 attachment, 3 obsolete attachments)

11.16 KB, patch
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review


10 years ago
window.blur() should send the window behind other windows (and does in Firefox), but in Camino it doesn't do anything.
Since this is another web-compat issue, nominating for 1.5.2, but it may be unreasonable to expect a patch.
Flags: camino1.5.2?
Not making 1.5.2, but for web-compat, we do want this whenever we get a patch.
Flags: camino1.6?
Flags: camino1.5.3?
Flags: camino1.5.2?
Flags: camino1.5.2-


10 years ago
Assignee: nobody → stuart.morgan
Ditto comment 2 for 1.5.4.
Flags: camino1.5.5?
Flags: camino1.5.4?
Flags: camino1.5.4-

Comment 4

10 years ago
Created attachment 292023 [details] [diff] [review]
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.
Assignee: stuart.morgan → nick.kreeger

Comment 5

10 years ago
+  if (window)
+      [window orderBack:nil];

Sorry bout the tab..

Comment 6

10 years ago
Created attachment 292026 [details] [diff] [review]
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 ;-)
Attachment #292023 - Attachment is obsolete: true
Attachment #292026 - Flags: review?(stuart.morgan)

Comment 7

10 years ago
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.
Attachment #292026 - Flags: review?(stuart.morgan) → review-
Minusing for 1.5.5; if there's a patch, we'll consider for 1.5.6 if it happens.
Flags: camino1.5.6?
Flags: camino1.5.5?
Flags: camino1.5.5-

Comment 9

10 years ago
Created attachment 302848 [details] [diff] [review]
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).
Attachment #292026 - Attachment is obsolete: true
Attachment #302848 - Flags: review?(stuart.morgan)

Comment 10

10 years ago
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]];
Attachment #302848 - Flags: review?(stuart.morgan) → review-

Comment 11

10 years ago
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

10 years ago
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?
Flags: camino1.6? → camino1.6-
Target Milestone: --- → Camino1.6

Comment 13

10 years ago
Created attachment 311205 [details] [diff] [review]

Taking back so that we can make sure this gets into 1.6. Thanks for doing all the leg work on this, Nick!
Assignee: nick.kreeger → stuart.morgan
Attachment #302848 - Attachment is obsolete: true
Attachment #311205 - Flags: superreview?(mikepinkerton)
Comment on attachment 311205 [details] [diff] [review]


+- (void)resignKeyAndOrderBack

can you add a function-comment on why this is needed?
Attachment #311205 - Flags: superreview?(mikepinkerton) → superreview+

Comment 15

10 years ago
Landed on trunk and MOZILLA_1_8_BRANCH, with a comment explaining that resignKeyAndOrderBack is for window.blur support.
Last Resolved: 10 years ago
Keywords: fixed1.8.1.13
Resolution: --- → FIXED
Camino 1.5.6 is not happening.
Flags: camino1.5.6? → camino1.5.6-
You need to log in before you can comment on or make changes to this bug.