113072 - Shift should unbackground a new tab

Status: VERIFIED FIXED

(SeaMonkey :: Tabbed Browser, enhancement, P3)

Description

[Wayne Davison]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.6) Gecko/20011120
BuildID:    2001112012

I was very pleasantly surprised when I discovered that adding a shift to a
middle-click (or ctrl-click) causes a tab to load in the background.  However,
when the "load in background" tabbed-browsing option is set, using the shift
modifier does not cause the tab to load in the foreground like I wish it would.

Reproducible: Always
Steps to Reproduce:
1.Turn on tabbed browsing for middle-click and set the load-in-background option.
2.Shift-middle-click a link.

Actual Results:  The link loads in the background, just like it would without
the shift.

Expected Results:  I'd like the link to load in the foreground when shift is used.

Comment 1

[R.K.Aa.]

-> Tabbed browser

Comment 2

[David Hyatt]

Reassigning to new component owner.

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

taking.  I have a fix for this.

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Proposed fix

We only want to load a tab in the foreground if the pref is on and the shift
key was clicked.  or the pref is off and shift is not clicked.	meaning both
true or both false.  Since we're dealing with 2 boolean values, we just compare
them.

Comment 5

[Doron Rosenberg (IBM)]

Comment on attachment 65972 [details] [diff] [review]
Proposed fix

r=doron

You might want to consider putting a comment about this, since a equality check
between a pref and a event is kinda confusing at first :)

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

>a equality check between a pref and a event is kinda confusing at first :)

Technically, I'm comparing, not checking for equality: There's a difference :)
But point taken.  I still think it is better than 

if ((event.shiftKey && pref.getBoolPref("browser.tabs.loadInBackground"))
    || (!event.shiftKey && !pref.getBoolPref("browser.tabs.loadInBackground")))

So, I can do this in both places:

+
+   // if event.shiftKey and pref.getBoolPref() are both true or both false
+   if (event.shiftKey == pref.getBoolPref("browser.tabs.loadInBackground"))


jag, will you sr= with the above comment in both places?

Comment 7

[sairuh (rarely reading bugmail)]

oopsey, almost forgot to cc marlon to see if he has any strong opinions here. :)

Comment 8

[jag (Peter Annema)]

Comment on attachment 65972 [details] [diff] [review]
Proposed fix

Nifty solution, but I think this is more readable and maintainable:

var loadInBackground = pref.getBoolPref("browser.tabs.loadInBackground");
if (event.shiftKey)
  loadInBackground = !loadInBackground;

if (!loadInBackground)
  ...

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch v2 - the "not-so-nifty"(tm) patch

Comment 10

[Doron Rosenberg (IBM)]

Comment on attachment 66135 [details] [diff] [review]
Patch v2 - the "not-so-nifty"(tm) patch

r=doron

Comment 11

[jag (Peter Annema)]

Comment on attachment 66135 [details] [diff] [review]
Patch v2 - the "not-so-nifty"(tm) patch

sr=jag

Comment 12

[Boris Zbarsky [:bzbarsky]]

checked into trunk

Comment 13

[sairuh (rarely reading bugmail)]

spiffy! verified fixed using 2002.01.29.1x comm bits on linux rh7.2, win2k and
mac 10.1.2.

added note to self: if/when we get around to writing a doc about modified mouse
actions, this nifty feature should be mentioned. also, on mac [or wherever the
middlemouse button isn't turned on for opening links], command+shift+click would
work in this case.