Closed
Bug 1175491
Opened 9 years ago
Closed 9 years ago
Eliminate FB toggle in FTU until new version of FB integration arrives
Categories
(Firefox OS Graveyard :: Gaia::First Time Experience, defect)
Firefox OS Graveyard
Gaia::First Time Experience
ARM
Gonk (Firefox OS)
Tracking
(b2g-master fixed)
RESOLVED
FIXED
FxOS-S2 (10Jul)
Tracking | Status | |
---|---|---|
b2g-master | --- | fixed |
People
(Reporter: borjasalguero, Assigned: fcampo)
References
Details
(Whiteboard: [NG Gaia Contacts][patch])
Attachments
(1 file)
No description provided.
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → fernando.campo
Updated•9 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → NGA S3 (26Jun)
Updated•9 years ago
|
Whiteboard: [NG Gaia Contacts]
Target Milestone: NGA S3 (26Jun) → FxOS-S1 (26Jun)
Assignee | ||
Comment 1•9 years ago
|
||
got rid of a bunch more fb related stuff than the toggle because of dependencies, and I don't usually like to leave code commented
Attachment #8625439 -
Flags: review?(sfoster)
Comment 2•9 years ago
|
||
I tested this patch out, and it *seems* to break gmail import. I'm getting an oauth frame not found kind of error. I've not dug to figure out why - I can probably look into it later today. I'll leave the review flag for now until I can confirm, but let me know if you see the same issue (or don't).
Flags: needinfo?(fernando.campo)
Updated•9 years ago
|
Whiteboard: [NG Gaia Contacts] → [NG Gaia Contacts][patch]
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Sam Foster [:sfoster] from comment #2)
> I tested this patch out, and it *seems* to break gmail import. I'm getting
> an oauth frame not found kind of error. I've not dug to figure out why - I
> can probably look into it later today. I'll leave the review flag for now
> until I can confirm, but let me know if you see the same issue (or don't).
Saw it, and I think it's solved now. I got carried away and deleted the iframe called 'fb-oauth', didn't realise it was used by other services too.
Flags: needinfo?(fernando.campo)
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8625439 [details] [review]
PR - https://github.com/mozilla-b2g/gaia/pull/30692
also asking Borja for feedback, as he is getting rid of facebook part in contacts settings part and we share code
Attachment #8625439 -
Flags: feedback?(borja.bugzilla)
Comment 5•9 years ago
|
||
Comment on attachment 8625439 [details] [review]
PR - https://github.com/mozilla-b2g/gaia/pull/30692
Looks like we also need to remove https://github.com/mozilla-b2g/gaia/blob/master/apps/ftu/js/services_launcher.js#L54,L60.
I'm a bit confused about ServicesLauncher.extensionFrame. The patch removes the stylesheet reference, but doesnt remove the CSS file. Do we actually need this iframe at all? Sorry I'm ignorant of how this works right now. It looks like its hooked into ServicesLauncher.open and ServicesLauncher.close which are called when we import outlook and gmail also.
Either way, I'd like to rename these services launcher elements - fb-curtain, fb-oauth and fb-extensions - its pretty confusing. I guess we did facebook oauth first then added the others. Now facebook is going away, we should call them importer-curtain importer-oauth or something like that.
From ftu.en-US.properties, we have quite a few facebook-related strings in there. Do these need to go away? I'm not clear on what the new plan will be but it seems to me we want to get a clean sheet here and add anything Facebook back in as/when necessary, so everything facebook should go in this patch.
We have facebook-related redirects in the manifest that look like they should be removed, and a datastores-access permission for Gaia_Facebook_Friends.
Attachment #8625439 -
Flags: review?(sfoster)
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8625439 [details] [review]
PR - https://github.com/mozilla-b2g/gaia/pull/30692
Works as expected in the device, but we need to clean the code removing all FB dependencies. We need to update as well the manifest file, as all 'redirect' and 'datastores' we had for FB are not going to be used anymore.
Some comments in Github to address, but definitely we are giving the right steps to remove all FB dependencies. \o/
Attachment #8625439 -
Flags: feedback?(borja.bugzilla) → feedback+
Updated•9 years ago
|
Target Milestone: FxOS-S1 (26Jun) → FxOS-S2 (10Jul)
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8625439 [details] [review]
PR - https://github.com/mozilla-b2g/gaia/pull/30692
updated PR, please re-review
- fb- names changed
- FB-related datastores and redirects erased
- CSS updated
- iframes renamed
Attachment #8625439 -
Flags: review?(sfoster)
Comment 8•9 years ago
|
||
Comment on attachment 8625439 [details] [review]
PR - https://github.com/mozilla-b2g/gaia/pull/30692
Looks good, thanks for the tidy-up.
Attachment #8625439 -
Flags: review?(sfoster) → review+
Assignee | ||
Comment 9•9 years ago
|
||
all green and autolander on holidays, so manual merge - https://github.com/mozilla-b2g/gaia/commit/a85322eb06b0787b0b8b2b34da1e4ce047c9ec92
thanks all for your comments
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
status-b2g-master:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•