Closed Bug 624028 Opened 9 years ago Closed 8 years ago

Provide easy-setup from mobile to desktop

Categories

(Cloud Services :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
mozilla10

People

(Reporter: philikon, Assigned: philikon)

References

Details

(Whiteboard: [verified in services])

Attachments

(6 files)

If your desktop machine dies or if you have two physically separate desktops, having a mobile connected to Sync can be handy if we exposed the Sync Key in the UI somehow (bug 620493) or, even better, allowed easy setup from the mobile to the desktop.

If we're fine with doing the typing on the mobile (right now you type the code into the device that sends credentials), this is purely a UI matter for Fennec. If we want to always have the desktop do the typing, we need to:

* implement receiveWithPIN and sendNoPIN methods in jpakeclient.js
* extend the Firefox UI to either show you a code or as you for one, depending on whether you're setting up from another desktop or a mobile
* provide an "Add a Device" UI on Fennec.
Duplicate of this bug: 647476
Bug 624028 already has a WIP patch, just FYI (especially rnewman)
Duplicate of this bug: 686478
Blocks: 675826
(Fleshy answer intended for Bug 686478.)

See

  https://wiki.mozilla.org/Firefox/Features/Sync_Setup_Improvements

which transitively links to 

  http://people.mozilla.com/~faaborg/files/projects/sync/allFlows-i2.png

which includes flows for setup in ways which we do not currently support.
Assignee: nobody → philipp
Attachment #568198 - Flags: review?(rnewman)
Attachment #568198 - Flags: feedback?(mark.finkle)
Excuse the awful colour scheme on the Fennec desktop build.
Comment on attachment 568198 [details] [diff] [review]
Part 1 (v1): Implement "Pair a Device" dialog

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

N.B., code review only; I haven't run this myself, because I'd be here all day waiting for a build.

Two personal opinions:

* I dislike the centered text in the wizard. The orphan ("device" in your screenshot) looks silly hanging out in the middle of the page.
* Seeing it in the flesh, so to speak, "Pair a Device" seems kinda lost and confusing on the home screen. No implication of *why*, or what this will involve.

::: mobile/chrome/content/sync.js
@@ +542,5 @@
>    }
>  };
> +
> +
> +const PIN_PART_LENGTH = 4;

Large parts of this section of the patch are identical to the contents of syncAddDevice.js.

They're not identical, but cross-linked comments would go a long way to avoid unintentional divergence in the future. ("Hey, look at syncAddDevice.js!".)

I just filed Bug 695879 to address one such divergence.

@@ +550,5 @@
> +
> +  open: function open() {
> +    this.code1.setAttribute("maxlength", PIN_PART_LENGTH);
> +    this.code2.setAttribute("maxlength", PIN_PART_LENGTH);
> +    this.code3.setAttribute("maxlength", PIN_PART_LENGTH);

Why do we use code[123] here, and pin[123] in syncAddDevice? Please either use 'pin' or file a followup to align syncAddDevice if you think that 'code' is significantly better.

(I err on the side of 'pin', given that we use PIN_PART_LENGTH...)
Attachment #568198 - Flags: review?(rnewman) → review+
Flagging for in-testsuite and in-litmus.  Will need testcases when this feature lands
Flags: in-testsuite?
Flags: in-litmus?(twalker)
Attachment #568198 - Flags: feedback?(mark.finkle) → feedback+
Attachment #568200 - Flags: review?(mark.finkle) → review+
Attachment #568201 - Flags: review?(mark.finkle) → review+
(In reply to Richard Newman [:rnewman] from comment #11)
> Two personal opinions:

... that I'm going to ignore (not because I don't agree with them but because we need to move this feature forward, and this is the UX we've got).

> > +  open: function open() {
> > +    this.code1.setAttribute("maxlength", PIN_PART_LENGTH);
> > +    this.code2.setAttribute("maxlength", PIN_PART_LENGTH);
> > +    this.code3.setAttribute("maxlength", PIN_PART_LENGTH);
> 
> Why do we use code[123] here, and pin[123] in syncAddDevice? Please either
> use 'pin' or file a followup to align syncAddDevice if you think that 'code'
> is significantly better.

The rest of the Fennec code uses 'code', so I stuck with that. In terms of "existing style predominates", the Fennec code is "closer" to this code than the Firefox code, IMHO.

Unless there are any other objections, I'm going to land the patches as is, given the r+ and feedback from you and mfinkle.
easy setup with s-c from mobile to desktop is good.  Pair a Device works from both home page and prefs
Whiteboard: [fixed in services] → [verified in services]
Duplicate of this bug: 707217
Status: RESOLVED → VERIFIED
Flags: in-litmus?(twalker) → in-litmus+
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.