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)

ARM
Gonk (Firefox OS)
defect
Not set
normal

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.
Assignee: nobody → fernando.campo
Status: NEW → ASSIGNED
Target Milestone: --- → NGA S3 (26Jun)
Whiteboard: [NG Gaia Contacts]
Target Milestone: NGA S3 (26Jun) → FxOS-S1 (26Jun)
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)
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)
Whiteboard: [NG Gaia Contacts] → [NG Gaia Contacts][patch]
(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)
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 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)
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+
Target Milestone: FxOS-S1 (26Jun) → FxOS-S2 (10Jul)
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 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+
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: