Opened 17 months ago

Last modified 10 days ago

#24920 needs_revision defect

TBA has tabs and private tabs, we only want private tabs

Reported by: sysrqb Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-mobile, TorBrowserTeam201906, ux-team
Cc: igt0, antonela Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Having both tabs and private tabs is confusing. We should remove all instances of non-private tabs.

Child Tickets

Attachments (1)

FirefoxNormalPrivate.png (47.9 KB) - added by sysrqb 13 days ago.
For comparison.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 17 months ago by gk

Keywords: tbb-mobile added

comment:2 Changed 17 months ago by sysrqb

Parent ID: 19675

comment:3 Changed 17 months ago by sysrqb

Parent ID: 19675#19675

comment:4 Changed 16 months ago by sysrqb

Parent ID: #19675#5709

Moving these to #5709, these aren't blockers anymore. We'll merge Orfox patches first, then audit everything as we move to m-c and work on TBA. (We should create more tickets as we find more items that need investigating/fixing)

comment:5 Changed 8 months ago by sysrqb

Summary: Orfox has tabs and private tabs, we only want private tabsTBA has tabs and private tabs, we only want private tabs

comment:6 Changed 5 weeks ago by gk

Parent ID: #5709

comment:7 Changed 2 weeks ago by sysrqb

Keywords: TorBrowserTeam201905R added
Status: newneeds_review

(It's almost June, but I'll be optimistic with the keyword :) )

Setting this for review, this is only for alpha - it's in my repo bug24920_01.

comment:8 Changed 2 weeks ago by gk

Keywords: TorBrowserTeam201906R added; TorBrowserTeam201905R removed

Moving reviews over to June.

comment:9 Changed 13 days ago by gk

Keywords: TorBrowserTeam201906 added; TorBrowserTeam201906R removed
Status: needs_reviewneeds_revision

Two things I am unsure about:

1) It's a bit sad to see the look of the browsing experience differ now from what we had before as it seems the private mode is differently styled (dark grey URL bar e.g.). This is not the case on desktop it seems? We get the same look regardless whether we have permanent private browsing mode on or off. I have not checked why that's the case, but I feel if possible then it is worth aligning the mobile and desktop experience here (and by that I mean that we'd ideally not make a visual difference indicating which mode we are on to not leak that information).

2) I spent a bit time trying to figure out why you do

+    let isPrivate = (("isPrivate" in aParams) && aParams.isPrivate) || Services.prefs.getBoolPref("browser.privatebrowsing.autostart")

Is that for defense-in-depth? Or is there an actual need for that? Ideally, I think the check for that pref (or, better: for whether we are in private browsing mode or not) should be done somewhere else if it is missing and not at this place (but that's a bit hard to say as I am not sure why it is needed), but it could get at least a comment explaining why it is there.

I am leaving this ticket open for addressing 2) and moved along with the changes meanwhile to get 9.0a2 built. I cherry-picked them to tor-browser-60.7.0esr-9.0-1 (commits 6d81ef66c55d5b88f39ee6527f8218b2f2e15cd8, 1666b0186346ab2769614d3b19bc2058fd6f13f0, 1bce86ec5cfff504baf6e61b9d356ff74c62b297, and 41b5dac4943dcf65582f2638fe6d1b5c80127c0a).

Could you please based your commits next time on whatever the current HEAD of the current tor-browser dev branch points to? I had to debug first why my testbuild was failing and it turned out that the Torbutton submodule commit you had on your branch did not point to any of the Torbutton commits in the repository but to one on a branch in my public Torbutton repo. Thus, the build was failing as the submodule update failed.

comment:10 in reply to:  9 ; Changed 13 days ago by sysrqb

Replying to gk:

Two things I am unsure about:

1) It's a bit sad to see the look of the browsing experience differ now from what we had before as it seems the private mode is differently styled (dark grey URL bar e.g.). This is not the case on desktop it seems? We get the same look regardless whether we have permanent private browsing mode on or off. I have not checked why that's the case, but I feel if possible then it is worth aligning the mobile and desktop experience here (and by that I mean that we'd ideally not make a visual difference indicating which mode we are on to not leak that information).

This is an interesting point. On desktop, the only visual indication of using a normal or private window is in the window's title bar (at least on Linux). The theme doesn't change, but on Android the equivalent of "Dark theme" is enabled when private tabs are used. Honestly, I'm not sure what we should do here. Following Mozilla's lead on this and using the dark theme with private tabs, like Fennec does, is the easy solution and tells the user "you are currently using private tabs". But, this should be the default in Tor Browser, so maybe that visual indicator isn't needed. As an alternative, we can use the "normal tab" colors with private tabs like tor browser on desktop, and if autostart is disabled we can return to using the two different color schemes.

2) I spent a bit time trying to figure out why you do

+    let isPrivate = (("isPrivate" in aParams) && aParams.isPrivate) || Services.prefs.getBoolPref("browser.privatebrowsing.autostart")

Is that for defense-in-depth? Or is there an actual need for that? Ideally, I think the check for that pref (or, better: for whether we are in private browsing mode or not) should be done somewhere else if it is missing and not at this place (but that's a bit hard to say as I am not sure why it is needed), but it could get at least a comment explaining why it is there.

Ah, yes, I can add a comment on this. It is needed for when extensions open their preferences page. These pages are opened entirely within the chrome javascript, so the Java part of this patch is never reached. By default, extensions open normal tabs for their preferences.

I am leaving this ticket open for addressing 2) and moved along with the changes meanwhile to get 9.0a2 built. I cherry-picked them to tor-browser-60.7.0esr-9.0-1 (commits 6d81ef66c55d5b88f39ee6527f8218b2f2e15cd8, 1666b0186346ab2769614d3b19bc2058fd6f13f0, 1bce86ec5cfff504baf6e61b9d356ff74c62b297, and 41b5dac4943dcf65582f2638fe6d1b5c80127c0a).

Could you please based your commits next time on whatever the current HEAD of the current tor-browser dev branch points to? I had to debug first why my testbuild was failing and it turned out that the Torbutton submodule commit you had on your branch did not point to any of the Torbutton commits in the repository but to one on a branch in my public Torbutton repo. Thus, the build was failing as the submodule update failed.

Sorry for this, yes, I'll be more careful about this in the future.

Changed 13 days ago by sysrqb

Attachment: FirefoxNormalPrivate.png added

For comparison.

comment:11 in reply to:  10 ; Changed 13 days ago by sysrqb

Replying to sysrqb:

This is an interesting point. On desktop, the only visual indication of using a normal or private window is in the window's title bar (at least on Linux).

Okay, that isn't entirely true. Private windows have a purple/white mask icon in the upper-right corner, too. There's limited space in the chrome on most Android phones, so I don't think adding another image like that is best option, either.

comment:12 in reply to:  11 Changed 12 days ago by gk

Replying to sysrqb:

Replying to sysrqb:

This is an interesting point. On desktop, the only visual indication of using a normal or private window is in the window's title bar (at least on Linux).

Okay, that isn't entirely true. Private windows have a purple/white mask icon in the upper-right corner, too. There's limited space in the chrome on most Android phones, so I don't think adding another image like that is best option, either.

Interesting, that's not what I see. I don't see any sign of private browsing mode on desktop at all. If I open my normal Firefox ESR and do open a new private window with, say, Ctrl+Shift+P then I get the result you describe but not in my Tor browser. Interestingly, if I open the menubar and go to File then the "New Private Window" option has the Ctrl+N shortcut which is traditionally only for opening normal Windows.

Last edited 12 days ago by gk (previous) (diff)

comment:13 Changed 12 days ago by Thorin

FYI: In FF (desktop), if you start in normal mode, then PB windows get the purple mask top right. If you start in PB Mode then all windows will be PB Mode (despite the menu item called New Window = effectively just a new PB window) and the purple mask is never shown. It's been this way since forever (that I've known).

Edit: that's the pref browser.privatebrowsing.autostart which is true for TB but default false for FF. Toggle that and restart the browser and redo your tests.

Last edited 12 days ago by Thorin (previous) (diff)

comment:14 Changed 12 days ago by pili

Cc: antonela added
Keywords: ux-team added

I wonder if we need some UX input here, what are the timelines for this? 9.0 stable?

comment:15 in reply to:  14 ; Changed 12 days ago by gk

Replying to pili:

I wonder if we need some UX input here, what are the timelines for this? 9.0 stable?

If we care about that it would be 9.0 stable, yes. I am still on the fence about how much effort we should spend here, though, given that Fenix is coming sooner than later...

comment:16 in reply to:  15 Changed 12 days ago by sysrqb

Replying to gk:

Replying to pili:

I wonder if we need some UX input here, what are the timelines for this? 9.0 stable?

If we care about that it would be 9.0 stable, yes. I am still on the fence about how much effort we should spend here, though, given that Fenix is coming sooner than later...

I think we should decide what UI/UX we want here, because I expect we'll carry this decision over to fenix, as well.

comment:17 Changed 11 days ago by antonela

Some comments:

+1 on keeping consistency between desktop and mobile. If the light theme is the default in desktop, we want to have the same default in mobile. Which of those themes (or even a custom one) are the default is a long discussion happening at #10399. So, for the TBA release could we have the same UX you got in the latest nightly but with the light/default theme?

Following Mozilla's lead on this and using the dark theme with private tabs, like Fennec does, is the easy solution and tells the user "you are currently using private tabs". But, this should be the default in Tor Browser, so maybe that visual indicator isn't needed.

I agree. The default in Tor Browser is a private tab + a lot of other things. Yesterday we talked about having or not the word "private" at the label menu items. Maybe, is redundant (and not accurate) to have New Private Tab in the Tor Browser. Just New Tab seems better. Could re-name New Private Tab to New Tab on the kebab menu?


Could we replace the mask icon with an onion icon? Would you let me know the sizes you need for that icon? Seems like the regular 16, 32, 64, 128 assets could do the job.

comment:18 in reply to:  17 Changed 10 days ago by gk

Replying to antonela:

Some comments:

+1 on keeping consistency between desktop and mobile. If the light theme is the default in desktop, we want to have the same default in mobile. Which of those themes (or even a custom one) are the default is a long discussion happening at #10399. So, for the TBA release could we have the same UX you got in the latest nightly but with the light/default theme?

Following Mozilla's lead on this and using the dark theme with private tabs, like Fennec does, is the easy solution and tells the user "you are currently using private tabs". But, this should be the default in Tor Browser, so maybe that visual indicator isn't needed.

I agree. The default in Tor Browser is a private tab + a lot of other things. Yesterday we talked about having or not the word "private" at the label menu items. Maybe, is redundant (and not accurate) to have New Private Tab in the Tor Browser. Just New Tab seems better. Could re-name New Private Tab to New Tab on the kebab menu?

#25660 might be relevant here. Where we had been thinking about "New Private Window" vs. "New Window".

Note: See TracTickets for help on using tickets.