Closed Bug 965811 Opened 10 years ago Closed 10 years ago

Support Firefox Account-based Sync in Send Tab activity

Categories

(Firefox for Android Graveyard :: Android Sync, defect)

Product:
Firefox for Android Graveyard
Component:
Android Sync
Version:
Firefox 29
Platform:
All
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox28 unaffected, firefox29 fixed, firefox30 fixed, firefox31 verified, fennec30+)

Status:
VERIFIED FIXED
Milestone:
Firefox 31
Tracking Flags:
Tracking Status
firefox28 --- unaffected
firefox29 --- fixed
firefox30 --- fixed
firefox31 --- verified
fennec 30+ ---

People

(Reporter: aaronmt, Assigned: rnewman)

References

Details

Attachments

(1 file)

Proposed patch. v1
57 bytes, text/x-github-pull-request
nalexander
: review+
lsblakk
: approval-mozilla-aurora+
lsblakk
: approval-mozilla-beta+
Details | Review 
In the browser share menu there is 'Firefox Sync' (classic) but no 'Firefox Account'.
This is straight forward, but not necessarily an Fx 29 thing.  We can get it cheaply, but I think we should wait until we get UX input on a better activity.  If I'm being really optimistic, we integrate push notifications and make this feature much more powerful before exposing it.
Will probably need a release note if it's gone for Fx29
Ian, what do you think we should do here?
tracking-fennec: ? → 30+
Flags: needinfo?(ibarlow)
We don't have a lot of Send Tab users, but I'm one of them, so…

Probably we can fix this by making the Send Tab activity the kernel of Bug 948509, and have it dispatch to the right place depending on which kind of Sync account you have set up.

I'm happy to take this on during Aurora.
Hardware: x86 → All
Using this bug to handle the rudiments of this: just bouncing to the right sync account depending on which one you have set up. This is basic parity.
Summary: Firefox Accounts - Missing share intent to send tab to device → Handle “Send Tab” for FxA-based Sync (the basics)
We're planning to pull "the basics" forward to 29+.
tracking-fennec: 30+ → 29+
(Noting in here that currently on 29 under Share is 'Firefox Sync', not sure if the plan is to append 'deprecated') or replace it all-together)
(In reply to Aaron Train [:aaronmt] from comment #7)
> (Noting in here that currently on 29 under Share is 'Firefox Sync', not sure
> if the plan is to append 'deprecated') or replace it all-together)

We'll call it either "Firefox Sync" or just "Firefox", I suspect. ibarlow gets that call :P
I think I am on Nick's side here, where we should take a more holistic look at this feature and how we really want it to work before moving it forward. Because I wouldn't sign off on the UX of its current implementation. I for one have actually never successfully sent a tab from one device to another. 

So whether we use better notifications, or have a better way to "catch" a tab on another device, or something, we should figure that out first and then release it.
Flags: needinfo?(ibarlow)
To be clear, Ian: we should remove this feature (for Old Sync and New Sync users) in 29?
That is what I am proposing, yes. Talked to Deb about it too, she agrees.
That makes me a sad panda. :-(
Morphing bug accordingly.
Summary: Handle “Send Tab” for FxA-based Sync (the basics) → Remove Send Tab functionality from Firefox 29
Keywords: user-doc-needed
Is this about killing "Send tab to device" and removing that info from our Support content? If so, I believe we never documented it in the first place...
More being prepared in case some of the few tens of thousands of users come to SUMO to find out why one of their QuickShare icons has gone away.
Got it. Thanks!

Ibai
Keywords: reproducible
(In reply to Sean McArthur [:seanmonstar] from comment #17)
> http://nooooooooooooooo.com

I feel the same way! On the plus side, the replacement will be better.
Not for Fx29, pushing to Fx30.
tracking-fennec: 29+ → 30+
This will need a release note.
Keywords: relnote
Send tabs is by far my favorite sync feature. I use it all the time, from all sorts of apps on my Android device. It's incredibly convenient to be able to decide that I really need to read this content on a desktop browser. Please don't take it away =[
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
I think engineering consensus has come to the viewpoint that:

1. Fixing this is just about as easy as removing it (indeed, it took me about half an hour)
2. If lots of people use it, removing it is bad; if nobody uses it, there's no harm in using it.

So:

https://github.com/mozilla-services/android-sync/pull/441
Summary: Remove Send Tab functionality from Firefox 29 → Support Firefox Account-based Sync in Send Tab activity
Attached file Proposed patch. v1 — 
That should be "no harm in leaving it".
Attachment #8397461 - Flags: review?(nalexander)
Tested with a Firefox Account, both the creation flow and sending.

Not (yet) tested with an old Sync account.
Verified with an old Sync account, too.
Comment on attachment 8397461 [details] [review]
Proposed patch. v1

That is a very clean implementation.
Attachment #8397461 - Flags: review?(nalexander) → review+
I was landing some stuff, so I landed this stuff too:

https://hg.mozilla.org/integration/fx-team/rev/35088bd669be
https://hg.mozilla.org/mozilla-central/rev/35088bd669be
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
needinfo for my uplift queue.
Flags: needinfo?(rnewman)
StrangeCharm: we'd love for you to test this for us.  Testing a Nightly 31 with this patch and an existing Sync 1.1 account would be valuable.  Testing a Nightly 31 with this patch and a Firefox Account + Sync 1.5 would be even more valuable (but I'm not sure you have such an account).

If user 'monreal' sees this, thanks for stopping by #androidsync.  Your feedback was valuable!
nalexander: I'd be happy to help if it were near my normal use. Testing this would be a lot of work, none of which is stuff I'd do anyway. Sorry.
status-firefox30: --- → affected
status-firefox31: --- → fixed
Flags: needinfo?(rnewman)
Target Milestone: --- → Firefox 31
Comment on attachment 8397461 [details] [review]
Proposed patch. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Addition of FxA Sync.

User impact if declined: 
  Users discovering the Send Tab feature while using FxA Sync will be prompted to pair Old Sync, which is crazy.

Testing completed (on m-c, etc.): 
  Hand-tested with both old and new accounts. Will be in tomorrow's Nightly, and will be verified there prior to uplift.

Risk to taking this patch (and alternatives if risky): 
  Fairly low. All changes are isolated to the Send Tab flow.

String or IDL/UUID changes made by this patch:
  None.
Attachment #8397461 - Flags: approval-mozilla-beta?
Attachment #8397461 - Flags: approval-mozilla-aurora?
Comment on attachment 8397461 [details] [review]
Proposed patch. v1

yes, let's please remove confusing things like this, pronto!
Attachment #8397461 - Flags: approval-mozilla-beta?
Attachment #8397461 - Flags: approval-mozilla-beta+
Attachment #8397461 - Flags: approval-mozilla-aurora?
Attachment #8397461 - Flags: approval-mozilla-aurora+
Aaron et al: could you please verify this on m-c prior to uplift? I'll be AFK this weekend.
Keywords: verifyme
If Aaron can look at this while it's on m-c, I'll grab a Nightly tomorrow and give it a spin, as well.
This WFM in the 2014-03-28 nightly on Android. Thanks for the fix!
Status: RESOLVED → VERIFIED
Works for me in Nightly 2014-03-28 as well.
Perfect, thanks for verifying.

Ready for uplift, then!
Keywords: verifyme
Whiteboard: [needs uplift]
I'll uplift.
Whiteboard: [needs uplift]
Got around to this today too, WFM. Missed the hoopla backchannel talk on wether this is shipping or not. From a functional perspective it WFM.
status-firefox31: fixed → verified
Product: Android Background Services → Firefox for Android
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: