183401 - Bring target tab to front on link drop, based on tab load-in-background setting

Status: RESOLVED FIXED

(Camino Graveyard :: Tabbed Browsing, defect)

Description

[Greg K.]

If the user has specified that tabs open in the foreground in the Preferences,
then dragging a link to a background tab should bring that tab to the foreground
as it does in Mozilla.

Otherwise, the current behavior, leaving the target tab in the background,
should persist.

Comment 1

[Mike Pinkerton (not reading bugmail)]

-> smfr

Comment 2

[Greg K.]

Note that Firebird also does this correctly (that is, like Mozilla), also, based
on its' load-links-in-background setting.

Comment 3

[Greg K.]

Consider for 0.8?

Comment 4

[Samuel Sidler (old account; do not CC)]

I agree. Targeting for 1.0.

Comment 5

[Samuel Sidler (old account; do not CC)]

Bump. This is important, but maybe not for 1.0 now. How simple is this to fix?

Comment 6

[Chris Lawson (gone)]

This ought to work:

    // if the user wants tabs to load in the background, DO NOT activate the URI when loading it
    BOOL loadInBG  = ![[PreferenceManager sharedInstance] getBooleanPref:"browser.tabs.loadInBackground" withSuccess:NULL];
    NSLog(@"Over a tabViewItem, will activate = %d", loadInBG);

(inserted at line 295 of BrowserTabView.mm), but for some reason, when I pass |loadURI:referrer:flags:activate:allowPopups| a YES in the activate: parameter, it doesn't work.

I wonder if that's a bug in the loadURI method on BrowserWrapper?

cl

Comment 7

[Chris Lawson (gone)]

Attachment: selects the tabViewItem that is the drag target if tabs load in foreground

I now see why I thought the loadURI method had a bug: the "activate" parameter doesn't actually activate the tab. From discussion on IRC, it seems that it puts focus in the content area.

I told Ian to look into re-naming that everywhere, and I'll file a separate bug for it.

cl

Comment 8

[Chris Lawson (gone)]

Targeting for 1.1 since we've got a patch now.

cl

Comment 9

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

Comment on attachment 223390 [details] [diff] [review]
selects the tabViewItem that is the drag target if tabs load in foreground

Shouldn't this respect the shift toggle, too?

Otherwise, it seems to work.

Comment 10

[Stuart Morgan]

Comment on attachment 223390 [details] [diff] [review]
selects the tabViewItem that is the drag target if tabs load in foreground

>+    // if the user wants tabs to load in the background, DO NOT select the tab that is the drag target after loading the URI
>+    BOOL shouldSelectTab  = ![[PreferenceManager sharedInstance] getBooleanPref:"browser.tabs.loadInBackground" withSuccess:NULL];
>+    if (shouldSelectTab)
>+      [self selectTabViewItem:overTabViewItem];

What if shift is currently held down?

If only there were some kind of central method somewhere that would do the right thing...

Comment 11

[Håkan Waara]

(In reply to comment #10)
> What if shift is currently held down?
> 
> If only there were some kind of central method somewhere that would do the
> right thing...

Yeah, that has been haunting us in so many bugs already!

I think that checking the current modifiers works, as long as you're not in a menu (in which case you ideally add an alternate menuitem). Chris, wanna try that in this patch?

Comment 12

[Chris Lawson (gone)]

Comment on attachment 223390 [details] [diff] [review]
selects the tabViewItem that is the drag target if tabs load in foreground

Obsolete. See bug 320668 for current patch.

cl

Comment 13

[froodian (Ian Leue)]

Chris, this was fixed by the checkin from bug 320668, right?

Comment 14

[Chris Lawson (gone)]

Affirmative, sir.