When link clickers visit an expired URL they should see an Expired Link page, rather than the Link Clicker UI

VERIFIED FIXED in Firefox 42

Status

P4
normal
Rank:
43
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: sevaan, Assigned: standard8)

Tracking

unspecified
mozilla42
x86
All
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +
in-testsuite +

Firefox Tracking Flags

(firefox42 verified)

Details

User Story

- Move the .spinner* classes from panel.css to common.css
- In StandaloneRoomInfoArea, we need to create a new div with classes 'spinner' and 'busy' that is displayed when the room state is INIT or GATHER.
- Change the rooms_unavailable_notification_message (search mxr, see below), to rooms_unavailable_notification_message2 and drop the "Sorry" from the string definition.
- Hide the retry button in the FAILED state if the failure details are EXPIRED_OR_INVALID
- Ensure the unit tests are updated (and/or update them as you go)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
As a link clicker, when visiting an expired url, I still see the Link Clicker UI with the button asking me to Join the Conversation. Only after clicking the conversation do I see text telling me the link has expired.

Desired result: Upon clicking an expired link I land on a page that right away tells me that the link has expired.
Hi Sevaan -- Do you have UX for this? (Or guidance on how you'd like it to look?)
backlog: --- → Fx38?
Flags: needinfo?(sfranks)
(Reporter)

Comment 2

4 years ago
Let's just replace the Join the conversation button (http://cl.ly/image/0L2E0U0x0t3C) with the text saying that the room has expired (http://cl.ly/image/213h1C0J2F1M), so the user see this without having to click.
Flags: needinfo?(sfranks)
(Assignee)

Comment 3

4 years ago
This most likely depends on bug 1114563 from a development perspective, as that will give us the information we need prior to entering the room.
Depends on: 1114563

Updated

4 years ago
Rank: 43
Priority: -- → P4

Updated

4 years ago
backlog: Fx38? → backlog+
Flags: firefox-backlog+
(Assignee)

Comment 4

3 years ago
Sevaan: I think we can do this change quite easily - we already attempt to get the room information, and that should tell us is a room is invalid or not.

A couple of questions - there's going to be a slight delay for getting the response to "join the conversation", on my machine its 200ms, but it could obviously be longer.

Do we want to have a spinner or some other temporary text? or just display nothing until we've got a response?

Additionally, is the existing text good enough if it is displayed up-front? here's the current text:

"Sorry, you cannot join this conversation. The link may be expired or invalid."

Lastly, presumably we wouldn't have the retry button any more?
(Reporter)

Comment 5

3 years ago
I think a spinner is probably a good idea. We should have some sort of feedback that information is pending and that would suffice. And yes, no retry button.

Matej, any thoughts on improving the text string, or it's okay as is?: 

"Sorry, you cannot join this conversation. The link may be expired or invalid."
Flags: needinfo?(matej)

Comment 6

3 years ago
(In reply to Sevaan Franks [:sevaan] from comment #5)
> I think a spinner is probably a good idea. We should have some sort of
> feedback that information is pending and that would suffice. And yes, no
> retry button.
> 
> Matej, any thoughts on improving the text string, or it's okay as is?: 
> 
> "Sorry, you cannot join this conversation. The link may be expired or
> invalid."

I'm going back on forth on whether we need the "sorry" or not, but otherwise I think it looks good.
Flags: needinfo?(matej)
(Reporter)

Comment 7

3 years ago
I vote for removing the sorry. It's not our fault and the less words the better.
(Assignee)

Comment 8

3 years ago
I'm happy to mentor someone on this bug. Experience with existing Firefox development processes would be useful - this is more complex than a simple good first bug, but should still be fairly easy.

See the user story for the list of things that need doing.

Background to loop development here: https://wiki.mozilla.org/Loop/Development

Note especially the standalone, react and unit test parts of that document.
Mentor: standard8
User Story: (updated)
Whiteboard: [lang=js][lang=react]
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1169763
Mark, this isn't exactly the same as 1169763, since a bogus URL should probably show up as different than an expired one.
(Assignee)

Comment 11

3 years ago
(In reply to Eric Rescorla (:ekr) from comment #10)
> Mark, this isn't exactly the same as 1169763, since a bogus URL should
> probably show up as different than an expired one.

We don't keep track of expired urls - it is either valid or it isn't, so there isn't a useful distinction that we can make.
(In reply to Mark Banner (:standard8) from comment #11)
> (In reply to Eric Rescorla (:ekr) from comment #10)
> > Mark, this isn't exactly the same as 1169763, since a bogus URL should
> > probably show up as different than an expired one.
> 
> We don't keep track of expired urls - it is either valid or it isn't, so
> there isn't a useful distinction that we can make.

I don't know how you're constructing URLs, but it wouldn't be difficult to add
a checksum that would allow you to distinguish these without having to keep
track.
(Assignee)

Comment 13

3 years ago
I initially thought this was a good mentored bug, but I investigated a bit during the plane flight for the work week and discovered it was much more complex that I expected. I have a patch in progress so will attach that in a bit.
Mentor: standard8
Whiteboard: [lang=js][lang=react]
(Assignee)

Updated

3 years ago
Blocks: 1154251
(Assignee)

Comment 14

3 years ago
Created attachment 8629884 [details] [diff] [review]
Make Loop's link-clicker show the expired/invalid failure view before the user clicks join.

This makes the store wait for the result of getting the server data before moving onto the READY state. This only affects standalone - as it is in the standalone part of the flow.

As a result of the waiting, we don't display the join button until we're in the READY state. If we hit an error, we go into the failure mode - for expired/invalid urls we'll no longer display the retry button.

I've also added a new action for the retry button, so that its easy to follow the flow, rather than attempting to mutate the JOIN case.

Various manual tests:
- Normal url should load fine & show the context and join
- Invalid url (e.g. change some of the token characters) should display an error message & no retry button.
- If testing with local servers, you can run up the loop-client but not the loop-server. Then when you load the page you get "Something went wrong" with a retry button.
- Also manually tested no media, by changing the if statement for hasDevices in activeRoomStore#joinRoom.
Attachment #8629884 - Flags: review?(mdeboer)
(Assignee)

Comment 15

3 years ago
Created attachment 8629900 [details] [diff] [review]
Make Loop's link-clicker show the expired/invalid failure view before the user clicks join. v2

Bitrot fix for latest fx-team.
Attachment #8629884 - Attachment is obsolete: true
Attachment #8629884 - Flags: review?(mdeboer)
Attachment #8629900 - Flags: review?(mdeboer)
(Assignee)

Updated

3 years ago
Assignee: nobody → standard8
Iteration: --- → 42.1 - Jul 13
Points: --- → 5
Flags: qe-verify+
Comment on attachment 8629900 [details] [diff] [review]
Make Loop's link-clicker show the expired/invalid failure view before the user clicks join. v2

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

Well done! This works just as I'd expect.

::: browser/components/loop/content/shared/js/activeRoomStore.js
@@ +163,5 @@
>  
>      /**
> +     * Attempts to retry getting the room data if there has been a room failure.
> +     */
> +    retryForRoomFailure: function() {

This name bugged me for some reason :)

How about just `retryGetRoomData`? Or `retryOnGetRoomFailure`?

@@ +595,5 @@
>          return;
>        }
>  
> +      var exitState = this._storeState.roomState !== ROOM_STATES.FAILED ?
> +        this._storeState.roomState : this._storeState.failureExitState;

nit: might as well swap this boolean condition around for legibility.
Attachment #8629900 - Flags: review?(mdeboer) → review+
https://hg.mozilla.org/mozilla-central/rev/81993d778728
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
I believe Anthony is working on Graphics now, so assigning to Bogdan who's in charge of testing Hello.
QA Contact: anthony.s.hughes → bogdan.maris
I verified using latest Nightly (2015-07-20) across platforms (Windows 7 64-bit, Mac OS X 10.10 and Ubuntu 14.04 32-bit) and pref 'loop.server' - 'https://loop-dev.stage.mozaws.net/v0' that error 'Sorry, you cannot join this conversation. The link may be expired or invalid.' is properly displayed without a 'Retry' button.
Status: RESOLVED → VERIFIED
status-firefox42: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.