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

VERIFIED FIXED in Firefox 34

Status

Hello (Loop)
Client
P1
normal
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: RT, Assigned: tOkeshu)

Tracking

unspecified
mozilla34
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox34 verified)

Details

(Whiteboard: p=1)

User Story

As a WebRTC browser but not on Firefox URL clicker, I want to be able to read the ToS and privacy notice before placing a call so that I know what I am signing-up for.

ToDo: Implement hiding the ToS and privacy link after the first call is made.

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Updated

4 years ago
User Story: (updated)
(Reporter)

Comment 1

4 years ago
"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.
(Reporter)

Updated

4 years ago
Priority: P2 → P1

Updated

4 years ago
Whiteboard: p=1
(Reporter)

Comment 2

4 years ago
UX reference: https://people.mozilla.org/~dhenein/labs/loop-spec-v1/
(Reporter)

Comment 3

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

Comment 4

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

Updated

4 years ago
Target Milestone: mozilla33 → mozilla34

Updated

4 years ago
Assignee: nobody → rgauthier
Target Milestone: mozilla34 → 34 Sprint 1- 8/4
Bug 1000127 is currently implementing these links.
Depends on: 1000127
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?
Flags: needinfo?(dhenein)
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.
Flags: needinfo?(dhenein)
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.
Ah, thanks for the explanation, Dan.  Adding Darrin in case he wants to chime in.
Target Milestone: 34 Sprint 1- 8/4 → 34 Sprint 2- 8/18
Sounds right to me -- as long as we show ToS/etc. once and then not after that.

Comment 11

3 years ago
Arcadio approved ToS URL to use https://hello.firefox.com/legal/terms
Status: NEW → ASSIGNED
User Story: (updated)
(Assignee)

Comment 12

3 years ago
Created attachment 8475926 [details] [diff] [review]
Hide TOS when it has been seen in the standalone UI
Attachment #8475926 - Flags: review?(dmose)
Comment on attachment 8475926 [details] [diff] [review]
Hide TOS when it has been seen in the standalone UI

Stealing review...
Attachment #8475926 - Flags: review?(dmose) → review?(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.
Attachment #8475926 - Flags: review?(standard8)
Attachment #8475926 - Flags: review-
Attachment #8475926 - Flags: feedback+
(Assignee)

Comment 15

3 years ago
Created attachment 8478255 [details] [diff] [review]
Hide TOS when it has been seen in the standalone UI
Attachment #8475926 - Attachment is obsolete: true
Attachment #8478255 - Flags: review?(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
Attachment #8478255 - Flags: review?(standard8) → review+
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.
(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.
https://hg.mozilla.org/integration/fx-team/rev/662567a5e509
Target Milestone: 34 Sprint 2- 8/18 → mozilla34
https://hg.mozilla.org/mozilla-central/rev/662567a5e509
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Is this sufficiently covered by automation or does it need manual testing?
Whiteboard: p=1 → p=1 [qa?]
(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?
Flags: qe-verify?
Flags: needinfo?(standard8)
Whiteboard: p=1 [qa?] → p=1
(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"
Flags: needinfo?(standard8)
Flags: qe-verify? → qe-verify+
I confirm this is working in the latest Aurora build. Sorry this fell off my radar.
Status: RESOLVED → VERIFIED
status-firefox34: --- → verified
QA Contact: anthony.s.hughes
You need to log in before you can comment on or make changes to this bug.