Closed Bug 1193765 Opened 5 years ago Closed 5 years ago

Share Link doesn't work from the user unavailable screen of direct calls

Categories

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

defect

Tracking

(firefox40 wontfix, firefox41 verified, firefox42 verified, firefox43 verified)

VERIFIED FIXED
mozilla43
Tracking Status
firefox40 --- wontfix
firefox41 --- verified
firefox42 --- verified
firefox43 --- verified

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: regression)

Attachments

(2 files)

I think we broke this with some of the context work.

STR

1) Try to call a contact that has logged into the system previously, and they either don't respond or are not connected at the time.
2) On the user unavailable screen, select "Share Link"

Actual Results

=> Failed to get a call url

Expected Results

=> Email window (or equivalent) pops up with a url to send.
Summary: Send Link doesn't work from the user unavailable screen of direct calls → Share Link doesn't work from the user unavailable screen of direct calls
This should fix it. The create function now requires the room name wrapped in decryptedContext, and also there's a bit of work to ensure that we add the room key onto the room url before returning it from create - we got away with it before because the push notification update was causing us to hit code that'd correct it for us.
Attachment #8646954 - Flags: review?(dmose)
Comment on attachment 8646954 [details] [diff] [review]
Share Link doesn't work from the user unavailable screen of direct calls.

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

Looks good; r=dmose.  Fixing the nits would be nice.

::: browser/components/loop/modules/LoopRooms.jsm
@@ +300,5 @@
>      let decryptedRoomKey = yield loopCrypto.decryptBytes(profileKey, encryptedKey);
>      return decryptedRoomKey;
>    }),
>  
> +  refreshRoomUrlWithNewKey: function(roomUrl, roomKey) {

JSDoc would be nifty.

::: browser/components/loop/test/xpcshell/test_looprooms.js
@@ +406,5 @@
>    let room = yield LoopRooms.promise("create", kCreateRoomProps);
> +
> +  // We can't check the value of the key, but check we've got a # which indicates
> +  // there should be one.
> +  Assert.ok(room.roomUrl.contains("#"), "Created room url should have a key");

You could also check that the key is a String with length > 0.
Attachment #8646954 - Flags: review?(dmose) → review+
This was a regression from the context in conversations work - bug 1142522
Approval Request Comment
[Feature/regressing bug #]: Bug 1142522
[User impact if declined]: If the user attempts a direct call to someone which fails, they cannot then use the share link button to conveniently send that person a link in an email.
[Describe test coverage new/current, TreeHerder]: Includes tests for the changed parts.
[Risks and why]: Low, small patch, Hello specific
[String/UUID change made/needed]: None
Attachment #8647453 - Flags: approval-mozilla-beta?
Attachment #8647453 - Flags: approval-mozilla-aurora?
Rank: 15
Flags: qe-verify+
Priority: -- → P1
https://hg.mozilla.org/mozilla-central/rev/633d7fba8154
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8647453 [details] [diff] [review]
Aurora/beta version of the patch.

Approving for uplift to Aurora since this is a mainline scenario. It also has automated tests which is great! 

Once this fix stabilizes a bit on Aurora, we can uplift to Beta as well.
Attachment #8647453 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8647453 [details] [diff] [review]
Aurora/beta version of the patch.

This has been in Nightly for a few days. Seems safe to uplift to Beta.
Attachment #8647453 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Reproduced the initial issue using old Nightly from 2015-08-12, verified that the issues is fixed now using Firefox 41 beta 3, latest Developer Edition 42.0a2 and latest Nightly 43.0a1 across platforms (Windows 7 64-bit, Mac OS X 10.10.4 and Ubuntu 14.04 32-bit).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.