Closed Bug 1081023 Opened 5 years ago Closed 5 years ago

Adjust standalone link clicker UI to handle change of URL format.

Categories

(Hello (Loop) :: Client, defect)

defect
Not set
Points:
1

Tracking

(firefox35 fixed, firefox36 fixed)

RESOLVED FIXED
mozilla36
Iteration:
36.1
Tracking Status
firefox35 --- fixed
firefox36 --- fixed
Blocking Flags:
backlog Fx34+

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Whiteboard: [standalone])

Attachments

(1 file, 1 obsolete file)

Bug 1046114 is going to change the url format that the server provides. We need to update the standalone UI to handle the new format, currently it relies on:

#call/<token>
backlog: --- → Fx34+
The main changes here are to the url handling in webapp.jsx. Most of the rest of the changes are consequential.

This makes it so that the standalone app can support urls of the format:

<hostname>/#call/{token}
<hostname>/c/{token}

as per bug 1046114 comment 33. It does not support the rooms format yet, as we haven't started the rooms work for standalone.

The changes in the server were mainly to try and be more explicit about which directories are hosted where on the server, to make maintenance easier. I also tried to document it, to make it easier in future.

Note that the old #call format will go away 30 days or so once we've transitioned the servers to the new format.
Attachment #8506888 - Flags: review?(nperriault)
Updated patch to account for bug 1079811 landing.

Setting review to NiKo or dmose.
Attachment #8507195 - Flags: review?(nperriault)
Attachment #8507195 - Flags: review?(dmose)
Comment on attachment 8507195 [details] [diff] [review]
Handle call url changes to the format for Loop's call links

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

Looks good but I feel like there's room for a tiny refactoring. As you're on PTO for 3 days, I'll take care of updating the patch.

::: browser/components/loop/standalone/content/js/webapp.js
@@ +929,1 @@
>      }

This should be extracted to some helper function, eg. extractLoopToken(document.location).

::: browser/components/loop/standalone/content/js/webapp.jsx
@@ +929,1 @@
>      }

How about moving this to some helper function so we could simply write:

var token = extractTokenFromLoopURL(document.location);
if (token) {
  conversation.set({loopToken: token});
}
Attachment #8507195 - Flags: review?(nperriault) → feedback+
Grmbl splinter. Oh well, you get the picture.
Comment on attachment 8506888 [details] [diff] [review]
Handle call url changes to the format for Loop's call links

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

Canceling review for this obsolete version of the patch.
Attachment #8506888 - Flags: review?(nperriault)
Whiteboard: [standalone]
Comment on attachment 8507195 [details] [diff] [review]
Handle call url changes to the format for Loop's call links

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

I'll actually r+ this, as working on a router impl. is being done in bug 1086588.
Attachment #8507195 - Flags: review?(dmose)
Attachment #8507195 - Flags: review+
Attachment #8507195 - Flags: feedback+
Removing checking-needed because of oranges: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c0aa34523ce8

Seems unrelated to current patch, but I'm no expert in this area, hence why removing checkin-needed. Any help appreciated.
Keywords: checkin-needed
Target Milestone: mozilla35 → ---
Attachment #8506888 - Attachment is obsolete: true
Try issues were unrelated (random failures), so pushed the patch:

https://hg.mozilla.org/integration/fx-team/rev/6f8bcb8400a5
Iteration: --- → 36.1
Points: --- → 1
Target Milestone: --- → mozilla36
https://hg.mozilla.org/mozilla-central/rev/6f8bcb8400a5
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Flags: qe-verify-
Flags: in-testsuite?
Comment on attachment 8507195 [details] [diff] [review]
Handle call url changes to the format for Loop's call links

Approval Request Comment
Landed on aurora per IRC with lsblakk with a=loop-only
Attachment #8507195 - Flags: approval-mozilla-aurora?
Attachment #8507195 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This is covered by unit tests and functional tests.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.