Bug 99202 – [Chromium] shift-click fails to spawn new window via window.open

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric U.]

[Chromium] shift-click fails to spawn new window with target=_blank

[Eric U.]

This is easy to test in GMail; any link in an email will always open in a new tab, even with shift held.  The correct behavior would be to spawn a new window if shift is held during the click.

[Eric U.]

Created attachment 168493 [details]
First try patch--no tests yet.

This code path affects <a target="_blank"...> and <a href="javascript:window.open...>, but apparently not ordinary links.

This didn't break any tests, which is a bad sign.  Where should I be putting a new test for this?

[Eric U.]

Digging a bit more, I see that this doesn't make e.g. middle-click on a javascript popup go to a background tab--it goes to a tab, but it's backgrounded.

E.g. "SEND TO A FRIEND" on http://www.yoshis.com/oakland/jazzclub/artist/show/3030

I haven't chased down why yet.

[Peter Kasting]

See also bug 22382, which is normally the cause of this sort of thing for middle-clicks.

[Eric U.]

(In reply to comment #4)
> See also bug 22382, which is normally the cause of this sort of thing for middle-clicks.

AhhhhhOK, I'm not going to chase middle-click right now, then.

I think this patch will just take care of shift-click in the broken case I know about, but I'd feel more comfortable if someone could suggest a test that it should be breaking.

[Eric U.]

OK, it's not target=_blank that does it--that works fine.  GMail is actually grabbing the event before that can hit, and using window.open, which is why we're hitting this code path.

Up until now, we've been ignoring much of the user gesture's information when we're in the window.open path; this patch changes that to allow shift and ctrl to override the default action more.  So now shift-click will work in GMail, but also if you shift-click on a button that spawns a popup, you'll get a new window [i.e. with toolbars, etc.] instead of a popup.

That seems like appropriate behavior to me.

[Eric U.]

(In reply to comment #3)
> Digging a bit more, I see that this doesn't make e.g. middle-click on a javascript popup go to a background tab--it goes to a tab, but it's backgrounded.
> 
> E.g. "SEND TO A FRIEND" on http://www.yoshis.com/oakland/jazzclub/artist/show/3030
> 
> I haven't chased down why yet.

(In reply to comment #3)
> Digging a bit more, I see that this doesn't make e.g. middle-click on a javascript popup go to a background tab--it goes to a tab, but it's backgrounded.
> 
> E.g. "SEND TO A FRIEND" on http://www.yoshis.com/oakland/jazzclub/artist/show/3030
> 
> I haven't chased down why yet.

Correction: that link now does what I expect it to with my change.  It's new-background-tab with middle-click and ctrl-click, new-foreground-tab with shift-middle-click.

[Eric U.]

Created attachment 170033 [details]
Same code, updated ChangeLog

The test for this code will be a Chromium browser_test; see https://codereview.chromium.org/11235048/ for the suite.
I've updated the ChangeLog to indicate this; the code is the same as before.

This should now be fully baked.

[Adam Barth]

Comment on attachment 170033 [details]
Same code, updated ChangeLog

Ok.

[WebKit Review Bot]

Comment on attachment 170033 [details]
Same code, updated ChangeLog

Clearing flags on attachment: 170033

Committed r132399: <http://trac.webkit.org/changeset/132399>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.