WIP feat(sync): choose what to sync on the web #2757
Conversation
vladikoff
force-pushed
the
choose-wat-to-sync
branch
from
a7c46f6
to
7663aba
Compare
vladikoff
force-pushed
the
choose-wat-to-sync
branch
3 times, most recently
from
5b7839e
to
882a9ba
Compare
| }; | ||
|
|
||
| return able.choose('chooseWhatToSyncMode', abData) === 'web' && flow === 'web'; | ||
| }, |
There was a problem hiding this comment.
@zaach @shane-tomlinson need feedback on the part above ^
vladikoff
force-pushed
the
choose-wat-to-sync
branch
from
882a9ba
to
223f6d4
Compare
| 'use strict'; | ||
|
|
||
| // old versions of Firefox do not support the CAPABILITIES message | ||
| // we timeout if that is the case | ||
| var CAPABILITIES_RENDER_TIMEOUT = 1500; |
There was a problem hiding this comment.
Out of curiosity, how did you pick this timeout value?
|
Thinking more broadly than just choose-what-to-sync, could this be a generic "initialize" event to which the browser responds as part of the page setup process? It could include its set of capabilities, its info about who is currently logged in (per #2662), and maybe other stuff in the future. I feel there's a bit of history with how we manage commands flowing back and forth between the page and the host, and I'm not the best person to advise on architecture details here. @shane-tomlinson will likely have valuable input, and I think we should run this past @zaach as it may play into his work on moving android to the web flow as well as #2662. @vladikoff do you have other potential capabilities in mind at this stage? (#2861 (comment) certainly qualifies, but maybe more?) |
| @@ -101,6 +102,19 @@ define([ | |||
| }, | |||
|
|
|||
| /** | |||
| * Check if the relier is Sync for Firefox Desktop | |||
| */ | |||
| isChooseWhatToSyncWeb: function (uniqueUserId, locationParams, able) { | |||
There was a problem hiding this comment.
Can this be moved to the fx-desktop relier since it's pretty specific to that relier?
|
|
||
| self.request(self._commands.CAPABILITIES) | ||
| .then(function (response) { | ||
| if (response && response.data && _.isObject(response.data)) { |
There was a problem hiding this comment.
is there always going to be a response.data? The request call usually resolves the promise with the data field. If there is a data field within that, then there is data.data and that seems strange and we definitely have to make sure all browsers send that format.
Riffing on this a little, and to @vladikoff's broader point about the complexity of implementing lots of custom brokers - can we use this kind of messaging mechanism to delegate all the broker customization points to the host? We have lots of custmization points like |
This is nearly identical to an idea discussed about a year ago, I think I referred to it as something like "querying the environment". The current terminology of "capabilities detection" is much clearer. I hope we can use this mechanism for all future browser integrations so we can stop creating custom brokers. We'll likely have to keep custom auth brokers around for existing browser integrations, because the browser won't respond to a As part of capabilities work, I think it would be nice if the browser specified its own Another place I could see this used - the browser could tell the content server - "no need to poll for signup verification, we have this." Overall, I'm an emphatic 👍 on the capabilities idea. I want us to get the API "mostly" right, making as few assumptions as possible, which is what I was trying to convey with my review comments. I think I came across as not liking the idea. Sorry @vladikoff, that was definitely not the intention. |
|
Seems like the approach in this PR is not gonna work. Will open a new PR |
Ref mozilla/fxa#18
depends on https://github.com/mozilla/fxa-content-experiments/pull/22/files
To test this you need a build of Firefox with patch applied: https://bugzilla.mozilla.org/show_bug.cgi?id=1183917
and adjust your signup url to force this:
('identity.fxaccounts.remote.signup.uri', fxaEnv.content + 'signup?service=sync&context=fx_desktop_v1&choose_what_to_sync_flow=web&force_choose_what_to_sync_view=web');