#24920: TBA has tabs and private tabs, we only want private tabs ----------------------------------------------+---------------------------- Reporter: sysrqb | Owner: tbb-team Type: defect | Status: | needs_revision Priority: Medium | Milestone: Component: Applications/Tor Browser | Version: Severity: Normal | Resolution: Keywords: tbb-mobile, TorBrowserTeam201906 | Actual Points: Parent ID: | Points: Reviewer: | Sponsor: ----------------------------------------------+----------------------------
Comment (by sysrqb): Replying to [comment:9 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. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24920#comment:10> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs