Closed Bug 1081731 Opened 6 years ago Closed 6 years ago

Add private browsing to Firefox OS in Gaia

Categories

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, enhancement, P2)

All
Gonk (Firefox OS)
enhancement

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S9 (21Nov)

People

(Reporter: fabrice, Assigned: kgrandon)

References

Details

(Whiteboard: [systemsfe])

Attachments

(2 files)

gecko support has landed, but we still need a way to let users open a browser frame in PB mode. 

UX ideas welcome!
I can see few points from which user can get into Private Browsing mode:

a) from 'cold' state (no browser window already opened) 
 a1) open browser window by the Browser icon
 a2) open web page by providing URL in Rocket Bar
b) browser is already opened (one or more windows empty or with content) 
c) user wish to open link from already opened web page content or from any other App which contains valid URL link

Propositions of solution:
a)
 a1) there is already Settings button in the right top corner where 'New Window' can be find among others, just add new option 'New Private Window'
 a2) the easiest way would be to add 'Private' button before 'Close' button (or maybe under 'Close' button to save space on devices with smaller screens), when user tap on 'Private' button it should change to 'Normal' or 'Non Private' button to allow user to change back to nonPB (nPB) if user wish to for any reason, maybe it would be also good to indicate for some way that user is in PB mode by red or yellow border around editable field of Rocket Bar or showing some drawing/string under Rocket Bar?
b) same as in a1) point - user opens new PB window by top right Settings menu from current browser window
c) currently when user taps on URL link nPB opens and such behavior should stay, but when user tap and hold the link then same menu opens as in a1) and in the menu 'New Private Window' can be found to open such link in PB instead of regular nPB mode with all its consequences
(In reply to mac from comment #1)
> I can see few points from which user can get into Private Browsing mode:
> 
> a) from 'cold' state (no browser window already opened) 
>  a1) open browser window by the Browser icon
>  a2) open web page by providing URL in Rocket Bar
> b) browser is already opened (one or more windows empty or with content) 
> c) user wish to open link from already opened web page content or from any
> other App which contains valid URL link
> 
> Propositions of solution:
> a)
>  a1) there is already Settings button in the right top corner where 'New
> Window' can be find among others, just add new option 'New Private Window'

This Settings button is gone in the new browser/search app. We now just have a button that switches to a card view with only the browser frames.

>  a2) the easiest way would be to add 'Private' button before 'Close' button
> (or maybe under 'Close' button to save space on devices with smaller
> screens), when user tap on 'Private' button it should change to 'Normal' or
> 'Non Private' button to allow user to change back to nonPB (nPB) if user
> wish to for any reason, maybe it would be also good to indicate for some way
> that user is in PB mode by red or yellow border around editable field of
> Rocket Bar or showing some drawing/string under Rocket Bar?

Right, but I fear we don't have enough space available for that on phones.

> b) same as in a1) point - user opens new PB window by top right Settings
> menu from current browser window

This just redirects to the rocketbar now.
Depends on: 1083975
(In reply to Fabrice Desré [:fabrice] from comment #2)
> (In reply to mac from comment #1)
> > I can see few points from which user can get into Private Browsing mode:
> > 
> > a) from 'cold' state (no browser window already opened) 
> >  a1) open browser window by the Browser icon
> >  a2) open web page by providing URL in Rocket Bar
> > b) browser is already opened (one or more windows empty or with content) 
> > c) user wish to open link from already opened web page content or from any
> > other App which contains valid URL link
> > 
> > Propositions of solution:
> > a)
> >  a1) there is already Settings button in the right top corner where 'New
> > Window' can be find among others, just add new option 'New Private Window'
> 
> This Settings button is gone in the new browser/search app. We now just have
> a button that switches to a card view with only the browser frames.

Bug 1083975 has been created to modify it to be able to handle change to new Private Browsing window

> >  a2) the easiest way would be to add 'Private' button before 'Close' button
> > (or maybe under 'Close' button to save space on devices with smaller
> > screens), when user tap on 'Private' button it should change to 'Normal' or
> > 'Non Private' button to allow user to change back to nonPB (nPB) if user
> > wish to for any reason, maybe it would be also good to indicate for some way
> > that user is in PB mode by red or yellow border around editable field of
> > Rocket Bar or showing some drawing/string under Rocket Bar?
> 
> Right, but I fear we don't have enough space available for that on phones.

Of course we have, I can see it on 3,5" screen of hamachi - there is enough space to put 6 more Rocket Bars!

> > b) same as in a1) point - user opens new PB window by top right Settings
> > menu from current browser window
> 
> This just redirects to the rocketbar now.

On my hamachi with latest FOTA applied it opens new Browser window like I tap on the Browser icon on Homescreen.
I've attached a pull request here which adds two entry points to private browsing in FirefoxOS. One being in the context menu when long tapping on any link, and the other being in the browser "overflow" menu at the top of every page.

This is a fairly well-hidden implementation, but if we land something like this first, it would allow us to iron out the UX a bit before introducing it more widely.
I tested basic cookie sharing between private and non-private browsing windows with <http://www.html-kit.com/tools/cookietester/>. They are not shared \o/
Ah, found a minor bug:
If I go to a website by entering a URL manually in the rocketbar, then its persist within the rocket bar even until I switched apps.

STR:
- Visit website in private tab / enter URL manually in rocket bar
- Press home
- Open browser app (this step may be optional)
- Touch rocket bar, you'll see the "private" URL here.
Thanks for the quick look Freddy. I'll take a look to see what we can do there.
Kevin,

What would you say to a tap/hold on the Search field on desktop/rocketbar/browser bringing the search field into "private" mode? Perhaps show an icon and color notification like Desktop FireFox does as well.
Another possible place to implement it would be in the Browsing Privacy Settings panel.
(In reply to Brett Edmond Carlock from comment #9)
> What would you say to a tap/hold on the Search field on
> desktop/rocketbar/browser bringing the search field into "private" mode?
> Perhaps show an icon and color notification like Desktop FireFox does as
> well.

Yes, I think there's definitely lots of ways that we could expose this more. We should also probably add a button from the initial browser page (or rocketbar) to go into private browsing. I do think it's probably best to land something basic first, although fairly obscure, and we can iterate on UX after it's landed. Thank you for the suggestions.
Comment on attachment 8521394 [details] [review]
[PullReq] KevinGrandon:bug_1081731_private_browsing to mozilla-b2g:master

I think there are many more UX improvements we can make here, though I think we should get something landed and iterate on it over the next few weeks.

Vivien/Etienne - Could you guys take a look at this and let me know what you think? Thanks!
Attachment #8521394 - Flags: review?(etienne)
Attachment #8521394 - Flags: review?(21)
Comment on attachment 8521394 [details] [review]
[PullReq] KevinGrandon:bug_1081731_private_browsing to mozilla-b2g:master

Looks solid! Left a comment on github but this is pretty much ready to land :)

Lets just file bugs for the missing pieces (button in the new tab app maybe? UX tweaks etc...) to make sure we're not forgetting anything.

(Worse case scenario if there's some blocking UX we can always hide it behind a setting.)
Attachment #8521394 - Flags: review?(etienne) → review+
(In reply to Etienne Segonzac (:etienne) from comment #13)
> Lets just file bugs for the missing pieces (button in the new tab app maybe?
> UX tweaks etc...) to make sure we're not forgetting anything.

Sounds good. I'll create a meta bug now and block that with the open issues.
Assignee: nobody → kgrandon
Whiteboard: [systemsfe]
Target Milestone: --- → 2.1 S9 (21Nov)
Blocks: 1098448
No longer depends on: 1083975
Comment on attachment 8521394 [details] [review]
[PullReq] KevinGrandon:bug_1081731_private_browsing to mozilla-b2g:master

Thanks for the review. Let's get this landed and tweak the UI/entry points over the course of 2.2 to see if we can ship this.
Attachment #8521394 - Flags: review?(21)
This has landed in master, and more polish to come.

https://github.com/mozilla-b2g/gaia/commit/12387a6a64e080005eca3b4d8eae9787760fb6ac
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Two questions about this landing.

1) Why are we using "Firefox OS" instead of a brand variable?

2) My understanding is that we're trying to move away from .innerHTML (CCing :stas to make sure that's true), so it's strange to see a new patch landing with it
One more note: there's a specific reason for splitting up long paragraphs in multiple strings (3 strings in Firefox become 1 in Firefox OS, for about 400 characters total). If you need to change a single word, the entire sentence will be invalidated and it needs to be translated again.
One more problem (sorry for the bugspam). Not sure if it makes sense to file a follow-up bug or just use this one given the potential issues.

I'm seeing a weird behavior: my locale is complete, but when I launch a new Private Window the entire window is not localized, it remains in English. But pseudolocales work just fine. Can someone else test this? If you want there's a fully translated master here for Italian: https://hg.mozilla.org/gaia-l10n/it
Here's a follow-up which updates the translation to use the brandShortName tag.

I'm more than happy to split this up into multiple strings if that would help. Stas - would it be better to use three strings instead of doing .innerHTML?
Flags: needinfo?(stas)
We'd like to avoid using innerHTML unless absolutely necessary (and until we have a better solution for those usecases).  It looks like we'll need to change this string anyways, so let's split it into smaller pieces. Maybe separate <p> elements for better semantics too?
Flags: needinfo?(stas)
Comment on attachment 8523718 [details] [review]
[PullReq] KevinGrandon:bug_1081731_locales_follow_up to mozilla-b2g:master

Francesco - do these changes look okay to you and could you give them a quick review?

As a side note I've also filed bug 1100298 about getting the proper content for the "Learn more." link.
Attachment #8523718 - Flags: review?(francesco.lodolo)
Comment on attachment 8523718 [details] [review]
[PullReq] KevinGrandon:bug_1081731_locales_follow_up to mozilla-b2g:master

Strings look definitely good to me, see comment 19 for testing (I can't get this window localized besides pseudolocales).
Attachment #8523718 - Flags: review?(francesco.lodolo) → review+
Thanks Francesco and Stas. Landed the follow-up: https://github.com/mozilla-b2g/gaia/commit/652c43f1e4e3bba547d524e8615b137946f57043

It also looks like adding the the manifest.webapp reference fixes translations for other "real" locales.
NIT, those strings should use curly quotes, instead of straight quotes
(In reply to Théo Chevalier [:tchevalier] from comment #26)
> NIT, those strings should use curly quotes, instead of straight quotes

I landed a follow-up to change the quote style: https://github.com/mozilla-b2g/gaia/commit/f004f8c891c4a6510a4679f3804ba3123af26615

Hopefully we don't need to update the keys for this change? I also wonder if we can make a linter for this?
(In reply to Kevin Grandon :kgrandon (In Europe/Conf until 11/24) from comment #27)
> Hopefully we don't need to update the keys for this change? I also wonder if
> we can make a linter for this?

No need to update keys (sorry for missing it, I never check apostrophes on other locales), linter sounds probably like a good idea.
Depends on: 1100549
You need to log in before you can comment on or make changes to this bug.