1019454 - Standalone UI for link clickers needs acceptance of ToS and privacy notice

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P1)

Comment 1

[Romain Testard [:RT]]

"By using [product] you agree to the terms of use and privacy notice" displayed at the bottom of the landing page for link clickers before the user presses the "Call" button.

No need to display this again once you are in a call.

Comment 2

[Romain Testard [:RT]]

UX reference: https://people.mozilla.org/~dhenein/labs/loop-spec-v1/

Comment 3

[Romain Testard [:RT]]

Link to ToS
https://github.com/mozilla/legal-docs/blob/master/WebRTC_ToS/en-US.md

Comment 4

[:shell escalante]

stand-alone UI: https://people.mozilla.org/~dhenein/labs/loop-link-spec/#call-start

Comment 5

[Mark Banner (:standard8)]

Bug 1000127 is currently implementing these links.

Comment 6

[OOO until Apr 20; Dan Mosedale (:dmosedale, :dmose)]

The only work to do here, I think, is to save the "seen" state in IndexedDb so users don't have to see it more than once, assuming that's part of the user story.  RT?

Comment 7

[Maire Reavy [:mreavy]]

Since RT is on PTO, I'm going to step in and say "yes" to Dan's question.  We want users to see the notice once and not be annoyed by it every time.   Dan -- Did you mean to needinfo RT instead of Darrin?  I'm going to remove the needinfo to Darrin.  Feel free to re-add if I'm mistaken.

Comment 8

[OOO until Apr 20; Dan Mosedale (:dmosedale, :dmose)]

mreavy: I needinfo-ed Darrin instead of RT because I knew that RT was going to be gone, and this is also a UX question.  That said, I'm sure your proxy answer is plenty sufficient, so I won't needinfo.

Comment 9

[Maire Reavy [:mreavy]]

Ah, thanks for the explanation, Dan.  Adding Darrin in case he wants to chime in.

Comment 10

[Darrin Henein [:darrin]]

Sounds right to me -- as long as we show ToS/etc. once and then not after that.

Comment 11

[:shell escalante]

Arcadio approved ToS URL to use https://hello.firefox.com/legal/terms

Comment 12

[Romain Gauthier [:tOkeshu]]

Attachment: Hide TOS when it has been seen in the standalone UI

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 8475926 [details] [diff] [review]
Hide TOS when it has been seen in the standalone UI

Stealing review...

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 8475926 [details] [diff] [review]
Hide TOS when it has been seen in the standalone UI

Review of attachment 8475926 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good and is almost there, but it needs the onunload removing, and the test sections merging.

::: browser/components/loop/standalone/content/js/webapp.jsx
@@ +557,5 @@
>      document.documentElement.lang = document.webL10n.getLanguage();
>      document.documentElement.dir = document.webL10n.getDirection();
> +
> +    window.onunload = function() {
> +      localStorage.setItem("has-seen-tos", "true");

I don't think we should set this on unload - it makes it very difficult to manually test / change the value and isn't a strict requirement.

From the user perspective, I could also see users taking a quick glance at a link, deciding not to make a call, and then wanting to make a call later (and hence not being able to look at the ToS in more detail).

::: browser/components/loop/test/standalone/webapp_test.js
@@ +703,5 @@
>        });
>      });
>    });
> +
> +  describe("StartConversationView", function() {

This seems to duplicate the existing StartConversationView section (maybe that landed recently?).

@@ +707,5 @@
> +  describe("StartConversationView", function() {
> +    var view, conversation, standaloneClientStub, oldLocalStorageValue;
> +
> +    beforeEach(function() {
> +      var oldLocalStorageValue = localStorage.getItem("has-seen-tos");

Duplicate var definition.

Comment 15

[Romain Gauthier [:tOkeshu]]

Attachment: Hide TOS when it has been seen in the standalone UI

Comment 16

[Mark Banner (:standard8)]

Comment on attachment 8478255 [details] [diff] [review]
Hide TOS when it has been seen in the standalone UI

Review of attachment 8478255 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great. r=Standard8

Comment 17

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Comment on attachment 8478255 [details] [diff] [review]
Hide TOS when it has been seen in the standalone UI

Review of attachment 8478255 [details] [diff] [review]:
-----------------------------------------------------------------

Just a quick comment about dependency management here.

::: browser/components/loop/standalone/content/js/webapp.jsx
@@ +202,5 @@
>      },
>  
>      componentWillUnmount: function() {
>        window.removeEventListener("click", this.clickHandler);
> +      localStorage.setItem("has-seen-tos", "true");

Let me jump in: I'd personally rather see this passed as a dependency through a prop, to ease testing and possibly switching backend later on.

Comment 18

[Mark Banner (:standard8)]

(In reply to Nicolas Perriault (:NiKo`) from comment #17)
> ::: browser/components/loop/standalone/content/js/webapp.jsx
> @@ +202,5 @@
> >      },
> >  
> >      componentWillUnmount: function() {
> >        window.removeEventListener("click", this.clickHandler);
> > +      localStorage.setItem("has-seen-tos", "true");
> 
> Let me jump in: I'd personally rather see this passed as a dependency
> through a prop, to ease testing and possibly switching backend later on.

Yeah, I can see that being useful. I don't see that being a significant benefit at the moment given the patch and amount of use.

If we get another item using local storage, then I think it'll be worth it, but at the moment I'm not expecting anything else to need this.

Comment 19

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/fx-team/rev/662567a5e509

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/662567a5e509

Comment 21

[u279076]

Is this sufficiently covered by automation or does it need manual testing?

Comment 22

[u279076]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #21)
> Is this sufficiently covered by automation or does it need manual testing?

Mark, can you please answer this?

Comment 23

[Mark Banner (:standard8)]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #21)
> Is this sufficiently covered by automation or does it need manual testing?

Probably manual testing for now. This should verify that the ToS and privacy link are displayed until a call is started.

The info is stored in localStorage, which can be reset by:

1) visit a call page.
2) Open the web console
3) Enter:

localStorage.removeItem("has-seen-tos");

4) It should then show on the console the line "undefined"

Comment 24

[u279076]

I confirm this is working in the latest Aurora build. Sorry this fell off my radar.