Closed Bug 1349155 Opened 7 years ago Closed 7 years ago

UX for shutting down in combination with clearing of user data

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: JanH, Assigned: JanH)

References

Details

Attachments

(3 files)

Because we now actually wait for sanitising of user data to finish before starting to shut down, there can now be a slight (or under unfavourable conditions not so slight) delay between pressing of the "Quit" button and the UI actually disappearing.

We could simply immediately send the UI into the background after pressing the button and then shutdown there, but
- that triggers the various onPause() handlers we have, which doesn't make the whole process any faster
- once backgrounded, we're liable to be killed by Android at any moment, even if we haven't finished sanitisation or our shutdown process yet
- if the user opens an external link in Firefox, the UI (while still shutting down) will pop up again and then disappear again as Gecko is terminating

so I'd rather not do that.

Ideally we could speed up sanitisation, but I'm not sure if and how easily that's possible. So in the meantime, maybe the best we could do is show a message (a snackbar, or something else?) that we're clearing up user data?
Flags: needinfo?(alam)
Depends on: 1345460
Hey JanH!

Thanks for filing this, I've noticed a bit of a delay too! 

Just to be clear, are you suggesting a Snackbar (or some sort of messaging) to indicate *immediately* to the user that something is happening, just before the app closes?

A sort of "Hey! we're  actually sanitizing your user data right now", and we'll close Firefox once that's complete!

Instead of the delay right now without any messaging?

I'm also going to CC product for prioritization.
Flags: needinfo?(alam)
oops, forgot to NI! ^
Flags: needinfo?(jh+bugzilla)
(In reply to Anthony Lam (:antlam) from comment #1)
> Just to be clear, are you suggesting a Snackbar (or some sort of messaging)
> to indicate *immediately* to the user that something is happening, just
> before the app closes?
> 
> A sort of "Hey! we're  actually sanitizing your user data right now", and
> we'll close Firefox once that's complete!
> 
> Instead of the delay right now without any messaging?

Exactly, that's what I had in mind.
Flags: needinfo?(jh+bugzilla)
This patch shows a snackbar with "Clearing user data..." if we're clearing any data during shutdown (can be tested here: https://queue.taskcluster.net/v1/task/EgCI7rmXQsibKdhQNsrLIg/runs/0/artifacts/public/build/target.apk). Let me know if you want to suggest a different wording or something else altogether...
Assignee: nobody → jh+bugzilla
Flags: needinfo?(alam)
Comment on attachment 8851373 [details]
Bug 1349155 - Show message when clearing user data on shutdown.

https://reviewboard.mozilla.org/r/123682/#review126252

::: mobile/android/locales/en-US/chrome/browser.properties:44
(Diff revision 1)
>  alertSearchEngineErrorToast=Couldn't add '%S' as a search engine
>  alertSearchEngineDuplicateToast='%S' is already one of your search engines
>  
> +# LOCALIZATION NOTE (alertShutdownSanitize): This text is shown as a snackbar during shutdown if the
> +# user has enabled "Clear private data on exit".
> +alertShutdownSanitize=Clearing user data...

nit: I think in other strings we use the ellipsis character (…) instead of three actual dots:
http://www.fileformat.info/info/unicode/char/2026/index.htm
Attachment #8851373 - Flags: review?(s.kaspari) → review+
(In reply to Jan Henning [:JanH] from comment #6)
> This patch shows a snackbar with "Clearing user data..." if we're clearing
> any data during shutdown (can be tested here:
> https://queue.taskcluster.net/v1/task/EgCI7rmXQsibKdhQNsrLIg/runs/0/
> artifacts/public/build/target.apk). Let me know if you want to suggest a
> different wording or something else altogether...

Thanks JanH! But sadly, I'm testing some other stuff on my Nightly build ATM so I can't install yours. 

Just as a final review, would it be possible to see a screenshot and a simple video of the experience of shutting down after this patch?
Flags: needinfo?(alam) → needinfo?(jh+bugzilla)
Voila.
Flags: needinfo?(jh+bugzilla)
Thanks JanH! 

sorry it took a while, this got buried in my email.

I think the last thing here is the copy. It could use a pass from our Content Lead so I'm going to leave an NI for her here and ping her tomorrow.
Flags: needinfo?(mheubusch)
Hi - thanks for looping me in.  Jan, can you tell me what user data you are clearing and why this is a good thing for a user? Is it to improve performance on subsequent sessions? Or to improve privacy? Or is this just getting rid of lingering "stuff" that built up during a user session - like, the user caused the stuff to build up by using the product and is considered user data because it was generated by the activity of the user?
Flags: needinfo?(mheubusch)
This would be shown if the user enables any of the items in Settings -> Privacy -> Clear private data on exit.

In that case a "Quit" button is added to the main menu, which triggers clearing of the selected data categories and then shuts down Firefox.
Because clearing data can take a noticeable amount of time in some cases [1], this introduces a delay between pressing "Quit" and any visible reaction (Firefox closes) happening. To avoid giving the impression that Firefox is simply hanging and not reacting, the idea was to show this message while data clearing was still in progress.


[1] Compare e.g. https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-04-10&keys=__none__!__none__!__none__&max_channel_version=beta%252F53&measure=FX_SANITIZE_TOTAL&min_channel_version=null&processType=*&product=Fennec&sanitize=1&sort_keys=submissions&start_date=2017-03-26&table=0&trim=1&use_submission_date=0
^ in case it didn't flag you Michelle :)
Flags: needinfo?(mheubusch)
Bump? It's still a bit until merge day, but I'd rather not leave it until the last minute...
Hi Jan - sorry for the delay.  I think the most important thing is to use the ellipsis in whatever messaging you use to indicate that something is happening (seriously, this is a thing).  I would also like to align the message with the setting, so ideally we would use:  Clearing private data. . .
Flags: needinfo?(mheubusch)
Thanks. I had no intention of getting rid of the ellipsis, and good idea about matching the text for the setting more closely.
hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 2 changes to 2 files
remote: error: pretxnchangegroup.a_treeclosure hook raised an exception: ''
remote: transaction abort!
remote: rollback completed
remote: ** unknown exception encountered, please report by visiting
remote: ** https://mercurial-scm.org/wiki/BugTracker
remote: ** Python 2.7.5 (default, Nov  6 2016, 00:28:07) [GCC 4.8.5 20150623 (Red Hat 4.8.5-11)]
remote: ** Mercurial Distributed SCM (version 4.1.2)
remote: ** Extensions loaded: blackbox, clonebundles, obsolescencehacks, pushlog, serverlog, readonly, vcsreplicator
remote: Traceback (most recent call last):
remote:   File "/var/hg/venv_pash/bin/hg", line 45, in <module>
remote:     mercurial.dispatch.run()
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 63, in run
remote:     sys.exit((dispatch(request(pycompat.sysargv[1:])) or 0) & 255)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 129, in dispatch
remote:     ret = _runcatch(req)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 219, in _runcatch
remote:     return callcatch(ui, _runcatchfunc)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 227, in callcatch
remote:     return scmutil.callcatch(ui, func)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/scmutil.py", line 152, in callcatch
remote:     return func()
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 208, in _runcatchfunc
remote:     return _dispatch(req)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 811, in _dispatch
remote:     cmdpats, cmdoptions)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 563, in runcommand
remote:     ret = _runcommand(ui, options, cmd, d)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 819, in _runcommand
remote:     return cmdfunc()
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 808, in <lambda>
remote:     d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/util.py", line 1051, in check
remote:     return func(*args, **kwargs)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/commands.py", line 5824, in serve
remote:     s.serve_forever()
remote:   File "/var/hg/version-control-tools/hgext/serverlog/__init__.py", line 320, in serve_forever
remote:     return super(sshserverwrapped, self).serve_forever()
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/sshserver.py", line 104, in serve_forever
remote:     while self.serve_one():
remote:   File "/var/hg/version-control-tools/hgext/serverlog/__init__.py", line 351, in serve_one
remote:     return super(sshserverwrapped, self).serve_one()
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/sshserver.py", line 122, in serve_one
remote:     rsp = wireproto.dispatch(self.repo, self, cmd)
remote:   File "/var/hg/version-control-tools/hgext/serverlog/__init__.py", line 343, in dispatch
remote:     return origdispatch(repo, proto, cmd)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/extensions.py", line 223, in closure
remote:     return func(*(args + a), **kw)
remote:   File "/var/hg/version-control-tools/pylib/vcsreplicator/vcsreplicator/hgext.py", line 359, in wireprotodispatch
remote:     return orig(repo, proto, command)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/wireproto.py", line 569, in dispatch
remote:     return func(repo, proto, *args)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/wireproto.py", line 982, in unbundle
remote:     proto._client())
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/exchange.py", line 1763, in unbundle
remote:     op = bundle2.processbundle(repo, cg, op=op)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/bundle2.py", line 356, in processbundle
remote:     _processpart(op, part)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/bundle2.py", line 433, in _processpart
remote:     handler(op, part)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/bundle2.py", line 1365, in handlechangegroup
remote:     ret = cg.apply(op.repo, 'bundle2', 'bundle2', expectedtotal=nbchangesets)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/changegroup.py", line 377, in apply
remote:     repo.hook('pretxnchangegroup', throw=True, **hookargs)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/localrepo.py", line 607, in hook
remote:     return hook.hook(self.ui, self, name, throw, **args)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/hook.py", line 199, in hook
remote:     res = runhooks(ui, repo, name, hooks, throw=throw, **args)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/hook.py", line 249, in runhooks
remote:     throw)
remote:   File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/hook.py", line 94, in _pythonhook
remote:     r = obj(ui=ui, repo=repo, hooktype=name, **args)
remote:   File "/var/hg/version-control-tools/hghooks/mozhghooks/treeclosure.py", line 70, in hook
remote:     return 0 if isPushAllowed(repo, treestatus_name) else 1
remote:   File "/var/hg/version-control-tools/hghooks/mozhghooks/treeclosure.py", line 37, in isPushAllowed
remote:     u = urlopen(url)
remote:   File "/usr/lib64/python2.7/urllib2.py", line 154, in urlopen
remote:     return opener.open(url, data, timeout)
remote:   File "/usr/lib64/python2.7/urllib2.py", line 431, in open
remote:     response = self._open(req, data)
remote:   File "/usr/lib64/python2.7/urllib2.py", line 449, in _open
remote:     '_open', req)
remote:   File "/usr/lib64/python2.7/urllib2.py", line 409, in _call_chain
remote:     result = func(*args)
remote:   File "/usr/lib64/python2.7/urllib2.py", line 1258, in https_open
remote:     context=self._context, check_hostname=self._check_hostname)
remote:   File "/usr/lib64/python2.7/urllib2.py", line 1217, in do_open
remote:     r = h.getresponse(buffering=True)
remote:   File "/usr/lib64/python2.7/httplib.py", line 1089, in getresponse
remote:     response.begin()
remote:   File "/usr/lib64/python2.7/httplib.py", line 444, in begin
remote:     version, status, reason = self._read_status()
remote:   File "/usr/lib64/python2.7/httplib.py", line 408, in _read_status
remote:     raise BadStatusLine(line)
remote: httplib.BadStatusLine: ''
abort: stream ended unexpectedly (got 0 bytes, expected 4)
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/464d9f5c2c3c
Show message when clearing user data on shutdown. r=sebastian
https://hg.mozilla.org/mozilla-central/rev/464d9f5c2c3c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: