826273 - Opening private tab and attempting to open a tab from last time opens it in a normal tab

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Paul [pwd]]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20130102 Firefox/20.0
Build ID: 20130102030907

Steps to reproduce:

This seems like a pretty huge issue to me. But opening a private tab opens about:home by default, clicking on a tab from last time will open the content in a normal tab as opposed to a private tab.

Comment 1

[Aaron Train [:aaronmt]]

As I understand it, this will resolve itself when about:home is not opened by default but rather about:privatebrowsing.

Comment 2

[Brian Nicholson (:bnicholson)]

(In reply to Aaron Train [:aaronmt] from comment #1)
> As I understand it, this will resolve itself when about:home is not opened
> by default but rather about:privatebrowsing.

Correct.

Comment 3

[Brian Nicholson (:bnicholson)]

Actually, this is still an issue even with about:privatebrowsing implemented since the user can still manually go to about:home. We need to make sure that tabs launched from about:home keep their privacy status.

Comment 4

[Brian Nicholson (:bnicholson)]

Attachment: Use private state of about:home tab for opened links

The easiest way to do this was to change loadUrlInTab() to loadUrl() with a private flag. This has two side effects:
1) "Tabs from last time" links will no longer reuse existing tabs of the same URL (not sure if we even wanted this anyway)
2) Tabs that are opened with "Tabs from last time" won't go back to about:home when the back button is pressed

Also, looking through this code, I noticed that we use inconsistent link behaviors for about:home. For thumbnails, we open them in the same tab; for "tabs from last time", we open them using loadUrlInTab(); and for synced tabs, we open them as standard new tabs.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 697694 [details] [diff] [review]
Use private state of about:home tab for opened links

>diff --git a/mobile/android/base/AboutHomeContent.java b/mobile/android/base/AboutHomeContent.java

>         mRemoteTabClickListener = new View.OnClickListener() {

>+                int flags = Tabs.LOADURL_NEW_TAB;
>+                if (Tabs.getInstance().getSelectedTab().isPrivate())
>+                    flags |= Tabs.LOADURL_PRIVATE;
>                 Tabs.getInstance().loadUrl((String) v.getTag(), Tabs.LOADURL_NEW_TAB);

You want to pass | flags | in loadUrl, right?

>                         container.setOnClickListener(new View.OnClickListener() {

>-                                Tabs.getInstance().loadUrlInTab(url);
>+                                int flags = Tabs.LOADURL_NEW_TAB;
>+                                if (Tabs.getInstance().getSelectedTab().isPrivate())
>+                                    flags |= Tabs.LOADURL_PRIVATE;
>+                                Tabs.getInstance().loadUrl(url, flags);

I have mixed feelings about losing the "if URL is already in a tab, use it" behavior, mainly because I hate ending up with duplicate tabs. If we really want this back, we can add a new flag to loadUrl(). I suppose losing the "parent" in this case is not a bad thing either.

r+ but fix the issue with | flags| not being used

Comment 6

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0e65de18e25

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f0e65de18e25

Comment 8

[Andreea Pod]

Verified fixed on:
-build: Firefox for Android 21.0a1 (2013-02-11)
-device: Samsung Galaxy Nexus 
-OS: Android 4.1.2

Comment 9

[Brian Nicholson (:bnicholson)]

Comment on attachment 697694 [details] [diff] [review]
Use private state of about:home tab for opened links

[Approval Request Comment]
Bug caused by (feature/regressing bug #): pb
User impact if declined: new tabs from private about:home don't open in private tabs
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 697694 [details] [diff] [review]
Use private state of about:home tab for opened links

Low risk, early in beta - and this seems like a worthwhile fix since about:home is also billed as Top Sites - quite likely users could hit this.

Comment 11

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/releases/mozilla-beta/rev/e2baa8dab050

Comment 12

[Adrian Tamas (:AdrianT)]

Verified on Aurora 21.0a2 2013-03-14 and Firefox Mobile 20 beta 5 on the Samsung Galaxy Tab 2 (Android 4.1.1)