Closed Bug 1140048 Opened 9 years ago Closed 3 years ago

Add "Send to device" Shareplane to top level menu

Categories

(Firefox for Android Graveyard :: General, defect, P2)

x86
Android
defect

Tracking

(firefox44 unaffected, firefox45 affected)

RESOLVED INCOMPLETE
Firefox 44
Tracking Status
firefox44 --- unaffected
firefox45 --- affected

People

(Reporter: antlam, Unassigned, Mentored)

References

Details

Attachments

(10 files, 7 obsolete files)

136.61 KB, image/png
Details
106.86 KB, image/png
Details
170.93 KB, image/png
Details
43.57 KB, image/jpeg
Details
6.88 KB, patch
dnirm
: review+
Details | Diff | Splinter Review
8.08 KB, patch
dnirm
: review+
Details | Diff | Splinter Review
3.97 KB, patch
dnirm
: review+
Details | Diff | Splinter Review
10.65 KB, patch
dnirm
: review+
Details | Diff | Splinter Review
6.38 KB, patch
dnirm
: review+
Details | Diff | Splinter Review
30.68 KB, image/png
antlam
: feedback+
Details
Attached image prev_sendto_mock1.png
Currently, our "Send to" action is under "share". I propose that we move it further up one level and place it next to the "Add to Reading List" icon. 

Goals: 
 - help us save users an extra press 
 - give this feature more exposure
 - further establish a cross platform use of the "Shareplane"
 - help with use-cases involving task continuity
Priority: -- → P1
Attached image prev_sendto_mock2.png
Attaching empty state (and half full/empty state) mocks.

Went through many options but this one gives us the benefit of a bit more context around what that button is. 

Still undecided if the share icon should end up on the right, or left (after the user shares an item). 

But otherwise, this is the design for the empty state.
via irc:

11:32 <mcomella> antlam: This is still something we want to do, right? [link to this bug]
11:34 <antlam> mcomella: I do, but not until we finish the confirmation & notification bugs
11:34 <antlam> The feedback ux needs to be complete
Talked to mcomella, we should be able to get this to track Firefox 44. Along with the polish feedback bug 1147653

NI-ing Barbara to update her on teh status.
Flags: needinfo?(bbermes)
Updated the corresponding Aha card! thanks everyone.
Flags: needinfo?(bbermes)
Assignee: nobody → michael.l.comella
tracking-fennec: --- → 44+
Dipti is going to take care of this (via bug 1166385 comment 65). I mentioned that this bug is tracking 44 and thus on a deadline.
Assignee: michael.l.comella → dnirm.moz
Hey, Dipti! In order to test sync, you'll need a Firefox account. Luckily for you, you can easily make a test Firefox account. To do so:
  * When you go to the "Create account" screen, type in an email address of the form "<user>@mockmyid.com". mockmyid.com is a special ending for test accounts. Continue to fill out the remaining fields (I'd recommend using a different password from your personal accounts)
  * Go to mailinator.com and type in <user> and click "Check it" - you'll get into a temporary mail account mailbox, the one your test account is registered to. For example, if I use "michael@mockmyid.com" on create account, I'd type in "michael" at mailinator.
  * Note: you'll only need to create an account for a mockmyid.com address once. After that, log in. (i.e. it's as you'd expect)
  * Verify the account via the email sent to that account and you should be good to go!
  * Sync another device to this account (i.e. log into it).
  * You'll want to make sure both devices sync to the account (it leans on Android which may not do this immediately). Go to 3-dot menu, Settings -> Sync -> Sync Now on both devices. Once they're synced, you should be able to send a tab.
  * To send a tab, click the 3-dot menu, click the Android share icon, and find "Send to other devices". iirc, it should always be at in the top few items of the menu.

Let me know if you have any questions!
(In reply to Michael Comella (:mcomella) from comment #7)
> Hey, Dipti! In order to test sync, you'll need a Firefox account. Luckily
> for you, you can easily make a test Firefox account. 
> 
> Let me know if you have any questions!

Thanks Michael for this info. Is there any specific reason for using test account? Recently I started using my private account for firefox sync across devices. I was wondering if I can use that here. Do I need to share the account details used?

I'm exploring the code for this feature and found [1] and ShareDialog as the starting point. 

[1] https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/overlays/service/sharemethods/SendTab.java#52

Should we have OverlayDialogButton for "send to device" like "add to reading list" and "bookmark" features?
Flags: needinfo?(michael.l.comella)
(In reply to Dipti Nirmale [:dnirm] from comment #8)
> Thanks Michael for this info. Is there any specific reason for using test
> account?

It separates personal data from test data, which some people like to do in case of corruption (which is unlikely in this case, but you can follow the better-safe-than-sorry approach).

For example, there was a Firefox for Android bug that I don't think was ever fixed where devices can be added to the list multiple times (e.g. add device to sync, remove device from sync, re-add device and it appears twice). When you're testing sync, it's possible you'll do these STR and litter your personal account with unused devices.

> Recently I started using my private account for firefox sync across
> devices. I was wondering if I can use that here.

Sure, if you like! Just mind the potential issues above.

> Do I need to share the account details used?

No.

> Should we have OverlayDialogButton for "send to device" like "add to reading
> list" and "bookmark" features?

No – sorry if I was unclear. We want to move the plane icon, which is "Send to other devices" button, from the Android share menu to the top-level menu – see the mock in comment 1.

> I'm exploring the code for this feature and found [1] and ShareDialog as the
> starting point. 

We want to start from some of the code that opens ShareDialog. I'll try to summarize what I think needs to happen:
  1) Add the share button to the quick share action bar [1] (3rd row). When you start digging, you'll notice the *same* share View is used twice – once to display the quick share bar and once to display the actual share button. They used to be in the same row, which is why they're the same view. When I split up the view, it was easier to re-use the View and hide the extraneous parts of it – essentially, you're to undo that work and put the share button in the same row as quick share. I believe I implemented this originally in [2].
  2) Replace the 2nd row share button in the menu with the send to other devices button. You can have it do nothing for now. The menu is defined in resources/menu*.
  3) Move the relevant "Send to other devices" code from ActivityChooserModel, GeckoActionProvider, etc. into the "Send to other devices button". Remove the rest. Note that the menu is initialized in BrowserApp (look at the "Add to reading list" button as an example, perhaps).

This is a pretty tricky set of changes! If there's something you don't understand, please let me know! This is a lot so focus on each part in isolation. If this seems like too much, I can help with some parts or provide further guidance.

NI to make sure you understand!

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/menu/GeckoMenu.java?rev=28d346b0142c#154
[2]: http://hg.mozilla.org/mozilla-central/rev/aadbc0a67a2c
Flags: needinfo?(michael.l.comella) → needinfo?(dnirm.moz)
(In reply to Michael Comella (:mcomella) from comment #9)
> We want to start from some of the code that opens ShareDialog. I'll try to
> summarize what I think needs to happen:
>   1) Add the share button to the quick share action bar [1] (3rd row). When
> you start digging, you'll notice the *same* share View is used twice – once
> to display the quick share bar and once to display the actual share button.
> They used to be in the same row, which is why they're the same view. When I
> split up the view, it was easier to re-use the View and hide the extraneous
> parts of it – essentially, you're to undo that work and put the share button
> in the same row as quick share. I believe I implemented this originally in
> [2].
>   2) Replace the 2nd row share button in the menu with the send to other
> devices button. You can have it do nothing for now. The menu is defined in
> resources/menu*.
>   3) Move the relevant "Send to other devices" code from
> ActivityChooserModel, GeckoActionProvider, etc. into the "Send to other
> devices button". Remove the rest. Note that the menu is initialized in
> BrowserApp (look at the "Add to reading list" button as an example, perhaps).
> 
> This is a pretty tricky set of changes! If there's something you don't
> understand, please let me know! This is a lot so focus on each part in
> isolation. If this seems like too much, I can help with some parts or
> provide further guidance.
> 
> NI to make sure you understand!
 
Thank you. This gives better understanding of what needs to be done. I'll let you know if I'm stuck somewhere. For now it looks good to start.
Flags: needinfo?(dnirm.moz)
Attachment #8665206 - Flags: review?(michael.l.comella)
Hey, Dipti – I'll try to get to this tomorrow.
Wanted to quickly bring up the design changes for the default "share" item in the menu (in comment 1) after moving the Shareplane out. 

Could I get an estimate of how achievable that is? Should we split it off into a second bug?
(In reply to Anthony Lam (:antlam) from comment #13)
> Could I get an estimate of how achievable that is? Should we split it off
> into a second bug?

T-shirt size small amount of work but I split it off into a separate bug for separation of concerns: bug 1208577.

Dipti, if you're interested, you can follow up with that bug after this one gets fixed.
Attached image share feature
Share button is replaced by 'Send to device' and share is added on third row in the previously submitted patch. Image shows share menu with and without the quick share items.

I couldn't understand how bug 1208577 is different. Also I am yet to work on 3rd point from Comment 9 (moving 'send to other devices' code). I found this part little tricky especially related to 'share - send to device'. Could you please point me to where I should look for?
Flags: needinfo?(michael.l.comella)
I'll get to your review Monday, Dipti.

(In reply to Dipti Nirmale [:dnirm] from comment #15)
> I couldn't understand how bug 1208577 is different.

In Anthony's mocks, the Android system share icon is combined with the "Share" text with 0 quick share items, and on the left side of the menu with >= 1 item, whereas your screenshot does not include it with 0 items, and puts it on the right with >= 1 item (this is how I was expecting this bug to go – looks good! :).

> Also I am yet to work on
> 3rd point from Comment 9 (moving 'send to other devices' code). I found this
> part little tricky especially related to 'share - send to device'. Could you
> please point me to where I should look for?

What do you mean by related to "share - send to device"?

I'm having difficulty providing guidance without more specific information on where you're stuck – should I just be more specific with what I said in comment 9?
Flags: needinfo?(michael.l.comella) → needinfo?(dnirm.moz)
(In reply to Michael Comella (:mcomella) from comment #16)
> (In reply to Dipti Nirmale [:dnirm] from comment #15) 
> > Also I am yet to work on
> > 3rd point from Comment 9 (moving 'send to other devices' code). I found this
> > part little tricky especially related to 'share - send to device'. Could you
> > please point me to where I should look for?
> 
> What do you mean by related to "share - send to device"?
> 
> I'm having difficulty providing guidance without more specific information
> on where you're stuck – should I just be more specific with what I said in
> comment 9?

Sorry. I should have been more specific. I spent some time looking for on click event handler for 'send to other device' through 'share'. Could you tell me which class handles this event?
Flags: needinfo?(dnirm.moz) → needinfo?(michael.l.comella)
(In reply to Dipti Nirmale [:dnirm] from comment #17)
> Sorry. I should have been more specific. I spent some time looking for on
> click event handler for 'send to other device' through 'share'. Could you
> tell me which class handles this event?

If you know the text of a string that is on a button you want to look up, you can search for the text to find the string name [1] to find where it is used [2] and then go through the results.

We actually just open the ShareDialog Activity [3] via a generic share menu callback listener.

[1]: https://mxr.mozilla.org/mozilla-central/search?string=send+to+other+device&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
[2]: https://mxr.mozilla.org/mozilla-central/search?string=overlay_share_send_other&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

[3]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/GeckoActionProvider.java?rev=a9738e5f636a#174
Flags: needinfo?(michael.l.comella)
Comment on attachment 8665206 [details] [diff] [review]
Replace share with 'send to device' in menu

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

Looks like you're going in the right direction ^_^ – you need to add the hit target, as you mentioned, and I'd like to rip more unused code out too!

If you're comfortable with version control, feel free to put these in separate commits.

::: mobile/android/base/BrowserApp.java
@@ +3012,5 @@
>  
>              GeckoActionProvider provider = GeckoActionProvider.getForType(GeckoActionProvider.DEFAULT_MIME_TYPE, this);
>  
> +            // For test. To be removed before commit
> +            provider.clearHistory();

If you have non-final code in your request, it should be a feedback request, not a review request.

btw, this doesn't compile for me.

::: mobile/android/base/menu/GeckoMenu.java
@@ +100,5 @@
>      // Map of "ifRoom" action-items in action-bar and their views.
>      private final Map<GeckoMenuItem, View> mSecondaryActionItems;
>  
> +    // Map of "ifRoom|withText" action-items in action-bar and their views.
> +    private final Map<GeckoMenuItem, View> mSecondaryTextActionItems;

nit: I'd prefer if this was called, `mShareActionItems`, or something. Same for the associated items below.

@@ +232,5 @@
> +                setAdapter(null);
> +                addHeaderView((DefaultActionItemBar) mSecondaryTextActionItemBar);
> +                setAdapter(mAdapter);
> +            }
> +            if (added = mSecondaryTextActionItemBar.addActionItem(actionView)) {

We should only add the header view if added is true here.

@@ +233,5 @@
> +                addHeaderView((DefaultActionItemBar) mSecondaryTextActionItemBar);
> +                setAdapter(mAdapter);
> +            }
> +            if (added = mSecondaryTextActionItemBar.addActionItem(actionView)) {
> +                    mSecondaryTextActionItems.put(menuItem, actionView);

nit: indentation.

::: mobile/android/base/menu/GeckoMenuItem.java
@@ +30,1 @@
>      public static final int SHOW_AS_ACTION_COLLAPSE_ACTION_VIEW = 8;

I think this is unused now.

@@ -140,5 @@
>      @Override
>      public View getActionView() {
>          if (mActionProvider != null) {
> -            if (getActionEnum() == MenuItem.SHOW_AS_ACTION_IF_ROOM) {
> -                return mActionProvider.onCreateActionView(SECONDARY_ACTION_BAR_HISTORY_SIZE,

These onCreateActionView(int, ActionViewType) methods are unused after your patch.

@@ -144,5 @@
> -                return mActionProvider.onCreateActionView(SECONDARY_ACTION_BAR_HISTORY_SIZE,
> -                        GeckoActionProvider.ActionViewType.DEFAULT);
> -            } else {
> -                return mActionProvider.onCreateActionView(QUICK_SHARE_ACTION_BAR_HISTORY_SIZE,
> -                        GeckoActionProvider.ActionViewType.QUICK_SHARE_ICON);

You can also remove the code using QUICK_SHARE_ICON (e.g. QuickShareBarActionView).

::: mobile/android/base/resources/menu-large-v11/browser_app_menu.xml
@@ +35,5 @@
>            android:showAsAction="ifRoom"/>
>  
> +    <item android:id="@+id/send_to_device"
> +        android:icon="@drawable/overlay_send_tab_icon"
> +        android:title="Send to device"

Use @string resources here – you can search for the entity name.

::: mobile/android/base/resources/menu-xlarge-v11/browser_app_menu.xml
@@ +42,4 @@
>      <item android:id="@+id/share"
>            android:icon="@drawable/ic_menu_share"
>            android:title="@string/share"
> +          android:showAsAction="ifRoom|withText"/>

I wasn't aware you could OR them. Nice, I like that because we'll actually be showing text. :P

::: mobile/android/base/resources/menu/browser_app_menu.xml
@@ +30,5 @@
>            android:icon="@drawable/ic_menu_new_private_tab"
>            android:title="@string/new_private_tab"/>
>  
> +    <item android:id="@+id/send_to_device"
> +          android:icon="@drawable/overlay_send_tab_icon"

Are these icons necessary? Have you tested on GB? I don't think these items appear in the top 5 items and thus don't have an icon.
Attachment #8665206 - Flags: review?(michael.l.comella) → review+
btw, you're going to make Anthony very happy. :)
(In reply to Michael Comella (:mcomella) from comment #18)
> (In reply to Dipti Nirmale [:dnirm] from comment #17)
> If you know the text of a string that is on a button you want to look up,
> you can search for the text to find the string name [1] to find where it is
> used [2] and then go through the results.

Thanks. I looked for 'send to device' instead 'send to other device'. I also got confused with the resource ids especially for overlay* though am getting familiar with them now :)
 
> 
> We actually just open the ShareDialog Activity [3] via a generic share menu
> callback listener. 
> [3]:
> https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/
> GeckoActionProvider.java?rev=a9738e5f636a#174

This is close to what I was looking for.
Initially I had thought of ShareDialog as starting point (Comment 8) but somehow missed [1] which actually sends tab to other devices. I'm curious on how gecko events are handled and would like to explore it later.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/overlays/ui/ShareDialog.java#395
(In reply to Michael Comella (:mcomella) from comment #19)
> Comment on attachment 8665206 [details] [diff] [review]
> Replace share with 'send to device' in menu
> 
> Review of attachment 8665206 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks like you're going in the right direction ^_^ – you need to add the hit
> target, as you mentioned, and I'd like to rip more unused code out too!
> 
> If you're comfortable with version control, feel free to put these in
> separate commits.

Should I add every class in separate commits? or should the code and resources be separated in different commits?

> ::: mobile/android/base/BrowserApp.java
> @@ +3012,5 @@
> >  
> >              GeckoActionProvider provider = GeckoActionProvider.getForType(GeckoActionProvider.DEFAULT_MIME_TYPE, this);
> >  
> > +            // For test. To be removed before commit
> > +            provider.clearHistory();
> 
> If you have non-final code in your request, it should be a feedback request,
> not a review request.
> 
> btw, this doesn't compile for me.

Sorry for this. I'll ensure the type of request onwards.

> 
> ::: mobile/android/base/resources/menu/browser_app_menu.xml
> @@ +30,5 @@
> >            android:icon="@drawable/ic_menu_new_private_tab"
> >            android:title="@string/new_private_tab"/>
> >  
> > +    <item android:id="@+id/send_to_device"
> > +          android:icon="@drawable/overlay_send_tab_icon"
> 
> Are these icons necessary? Have you tested on GB? I don't think these items
> appear in the top 5 items and thus don't have an icon.

I'm not sure on this. What is the criteria for adding the icons in menu? Could you also tell me what GB is?
Flags: needinfo?(michael.l.comella)
(In reply to Michael Comella (:mcomella) from comment #20)
> btw, you're going to make Anthony very happy. :)

I hope it makes into final release then :)
(In reply to Dipti Nirmale [:dnirm] from comment #21)
> (In reply to Michael Comella (:mcomella) from comment #18)
> > (In reply to Dipti Nirmale [:dnirm] from comment #17)
> > If you know the text of a string that is on a button you want to look up,
> > you can search for the text to find the string name [1] to find where it is
> > used [2] and then go through the results.
> 
> Thanks. I looked for 'send to device' instead 'send to other device'. I also
> got confused with the resource ids especially for overlay* though am getting
> familiar with them now :)

Could you recommend any tool which maps elements on android ui to its id or layout? I generally do it intuitively which may not be the right way. I found [1] on similar lines.

[1]: http://stackoverflow.com/questions/17354371/is-there-an-equivalent-to-dom-inspector-for-android-native-layouts
Awesome! Thank you Dipti!

Two nits:
 - Can we resize the plane in the top menu now? I want to try and shrink it a bit cause it looks kinda large. 24dp max height should do it.

 - When there are no items in the quick share yet, we have  > there right now, can we change that to the share icon so there are cross-state consistencies?
Flags: needinfo?(dnirm.moz)
(In reply to Anthony Lam (:antlam) from comment #25)
> Awesome! Thank you Dipti!
> 
> Two nits:
>  - Can we resize the plane in the top menu now? I want to try and shrink it
> a bit cause it looks kinda large. 24dp max height should do it.

Can you please confirm if you mean second row (for send to other device)? I'm somewhat naive to UI design :P. Is the plane changed because of the icon used? I see there are two icons (a)icon_shareplane.png and (b)overlay_send_tab_icon.png among which I've used the (b) for send to device. Should I use the other one? 

I have one query regarding the behaviour of this added menu. What should happen after tapping 'Send to other device' in case no sync account or device exists? I think this menu should be disabled in such case. The current behaviour seems not to display this option on 'share'. 

>  - When there are no items in the quick share yet, we have  > there right
> now, can we change that to the share icon so there are cross-state
> consistencies?

I am not sure how feasible this is (from [1]). We might get some work-around after some digging. I see follow-up bug 1208577 for the same from Comment 16. Should we do it after this bug is fixed?

[1] : http://ux.stackexchange.com/questions/57423/why-does-android-not-show-an-icon-in-the-overflow-menu
Flags: needinfo?(dnirm.moz)
Flags: needinfo?(alam)
(In reply to Dipti Nirmale [:dnirm] from comment #26)
> (In reply to Anthony Lam (:antlam) from comment #25)
> > Awesome! Thank you Dipti!
> > 
> > Two nits:
> >  - Can we resize the plane in the top menu now? I want to try and shrink it
> > a bit cause it looks kinda large. 24dp max height should do it.
> 
> Can you please confirm if you mean second row (for send to other device)?
> I'm somewhat naive to UI design :P. Is the plane changed because of the icon
> used? I see there are two icons (a)icon_shareplane.png and
> (b)overlay_send_tab_icon.png among which I've used the (b) for send to
> device. Should I use the other one? 

No worries! Yes, I'm referring to the plane icon in your comment 15 attachment.

Nope, it's the same plane icon and we're not changing it so we can just resize the current asset.

> I have one query regarding the behaviour of this added menu. What should
> happen after tapping 'Send to other device' in case no sync account or
> device exists? I think this menu should be disabled in such case. The
> current behaviour seems not to display this option on 'share'. 
> 
> >  - When there are no items in the quick share yet, we have  > there right
> > now, can we change that to the share icon so there are cross-state
> > consistencies?
> 
> I am not sure how feasible this is (from [1]). We might get some work-around
> after some digging. I see follow-up bug 1208577 for the same from Comment
> 16. Should we do it after this bug is fixed?
> 
> [1] :
> http://ux.stackexchange.com/questions/57423/why-does-android-not-show-an-
> icon-in-the-overflow-menu

Sure, works for me
Flags: needinfo?(alam) → needinfo?(dnirm.moz)
Attached patch Remove QUICK_SHARE_ICON (obsolete) — Splinter Review
Do not display Send to other device as sub menu in share
Attachment #8668067 - Flags: review?(michael.l.comella)
Attachment #8668070 - Flags: review?(michael.l.comella)
Attachment #8665206 - Attachment is obsolete: true
Attachment #8668072 - Flags: review?(michael.l.comella)
QA Contact: teodora.vermesan
(In reply to Anthony Lam (:antlam) from comment #27)
> (In reply to Dipti Nirmale [:dnirm] from comment #26)
> > (In reply to Anthony Lam (:antlam) from comment #25)
> > >  - Can we resize the plane in the top menu now? I want to try and shrink it
> > > a bit cause it looks kinda large. 24dp max height should do it.
> > 
> > Can you please confirm if you mean second row (for send to other device)?
> > I'm somewhat naive to UI design :P. Is the plane changed because of the icon
> > used? I see there are two icons (a)icon_shareplane.png and
> > (b)overlay_send_tab_icon.png among which I've used the (b) for send to
> > device. Should I use the other one? 
> 
> No worries! Yes, I'm referring to the plane icon in your comment 15
> attachment.
> 
> Nope, it's the same plane icon and we're not changing it so we can just
> resize the current asset.

I tried setting the height of the plane to 24dp in [1], [2] & [3] but it did not change the view. Probably Michael could help here. If it is icon design specific requirement, can we have a new bug for this?

[1] : https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/menu_item_action_view.xml#24
[2] : https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/menu_secondary_action_bar.xml#13
[3] : https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values/styles.xml#95
Flags: needinfo?(dnirm.moz) → needinfo?(alam)
(In reply to Dipti Nirmale [:dnirm] from comment #22)
> Should I add every class in separate commits? or should the code and
> resources be separated in different commits?

Whatever is most intuitive. I usually start by adding my png resources in a separate commit, and will separate by building small features. Where it goes from there is dependent on how I write the code.

> Sorry for this. I'll ensure the type of request onwards.

No need to apologize – that's how we learn! :)

> Could you also tell me what GB is?

Gingerbread, which is the codename for Android 2.3.

> > Are these icons necessary? Have you tested on GB? I don't think these items
> > appear in the top 5 items and thus don't have an icon.
> 
> I'm not sure on this. What is the criteria for adding the icons in menu?

On GB, it's the top five items, I believe. For example, see this menu [1]. Notably, item 6 on our menu is more, which brings up a list without icons.

If you don't have a GB device, I can test for you – just NI me.

[1]: https://encrypted.google.com/search?q=gingerbread%20menu&hl=en&client=firefox-a&hs=jN1&rls=org.mozilla:en-US:unofficial&prmd=imvns&source=lnms&tbm=isch&ei=OiInT-qcI8nygge53t3wCQ&sa=X&oi=mode_link&ct=mode&cd=2&ved=0CBMQ_AUoAQ&biw=1369&bih=967#imgrc=U4PIpJHZPtw1IM%3A&oi=mode_link
Flags: needinfo?(michael.l.comella)
(In reply to Dipti Nirmale [:dnirm] from comment #24)
> Could you recommend any tool which maps elements on android ui to its id or
> layout? I generally do it intuitively which may not be the right way. I
> found [1] on similar lines.

Hierarchy Viewer would be the only one I know of, but it's not as fast as the DOM inspector. I generally read the code.
(In reply to Dipti Nirmale [:dnirm] from comment #32)
> I tried setting the height of the plane to 24dp in [1], [2] & [3] but it did
> not change the view. Probably Michael could help here. If it is icon design
> specific requirement, can we have a new bug for this?

I'm not sure what you mean by "icon design specific requirement" but I'll take a look into this and see what I can find.
Comment on attachment 8668066 [details] [diff] [review]
Add 'send to device' and remove quick share from menu resource

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

It's good to have your patches stand on their own (e.g. this will fail to compile because the code tries to access R.id.quickshare) so someone can build each part of your code or check out any revision and still compile, but it's okay to do it this way occasionally. Thanks for breaking this apart!

r+ w/ nits.

::: mobile/android/base/resources/menu-v11/browser_app_menu.xml
@@ +42,5 @@
>      <item android:id="@+id/share"
>            android:icon="@drawable/ic_menu_share"
>            android:title="@string/share"
> +          android:showAsAction="ifRoom|withText">
> +    </item>

Use `/>` rather than a separate closing tag.

::: mobile/android/base/resources/menu/browser_app_menu.xml
@@ +36,2 @@
>      <item android:id="@+id/share"
> +          android:icon="@drawable/ic_menu_share"

I don't think this icon is necessary either.
Attachment #8668066 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8668067 [details] [diff] [review]
Remove QUICK_SHARE_ICON

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

This patch does a lot more than "Remove QUICK_SHARE_ICON" – you should try to make the patches bite-sized enough to properly represent whatever the commit asks for. I'm not fine leaving it as is now to prevent wasting time reorganizing working commits.

r+ w/ nits.

::: mobile/android/base/widget/ActivityChooserModel.java
@@ -781,5 @@
> -                 */
> -                if (shareDialogClassName.equals(resolveInfo.activityInfo.name) &&
> -                        channelToRemoveLabel.equals(resolveInfo.loadLabel(packageManager))) {
> -                    // Don't add the menu item if there are no devices to share to.
> -                    if (!hasOtherSyncClients()) {

This method is unused.

::: mobile/android/base/widget/GeckoActionProvider.java
@@ -102,5 @@
>              case DEFAULT:
>                  view = new MenuItemActionView(mContext, null);
>                  break;
>  
> -            case QUICK_SHARE_ICON:

Don't forget to remove this from the enum.

@@ -156,1 @@
>          final String sendTabLabel = mContext.getResources().getString(R.string.overlay_share_send_other);

This and shareDialogClassName are unused.

Nice find with removing this!
Attachment #8668067 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8668070 [details] [diff] [review]
Replace quick share view with share in menu

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

r+ w/ nits.

::: mobile/android/base/menu/GeckoMenu.java
@@ +233,5 @@
> +                    setAdapter(null);
> +                    addHeaderView((DefaultActionItemBar) mShareActionItemBar);
> +                    setAdapter(mAdapter);
> +                }
> +                    mShareActionItems.put(menuItem, actionView);

nit: indentation
Attachment #8668070 - Flags: review?(michael.l.comella) → review+
(In reply to Dipti Nirmale [:dnirm] from comment #26)
> I have one query regarding the behaviour of this added menu. What should
> happen after tapping 'Send to other device' in case no sync account or
> device exists? I think this menu should be disabled in such case. The
> current behaviour seems not to display this option on 'share'.

Anthony and I will get back to you regarding this – we've discussed this before but misplaced the notes we wrote.
Comment on attachment 8668067 [details] [diff] [review]
Remove QUICK_SHARE_ICON

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

You also have to remove QuickShareBarActionView.java from m/a/b/moz.build.
moz.build lists all of the files we include in our builds via `./mach` (as opposed to Intellij which unfortunately uses a different build system at the moment).
Comment on attachment 8668067 [details] [diff] [review]
Remove QUICK_SHARE_ICON

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

::: mobile/android/base/widget/ActivityChooserModel.java
@@ -778,5 @@
> -                 *
> -                 * Note: we check both the class name and the label to ensure we only change the
> -                 * label of the current channel.
> -                 */
> -                if (shareDialogClassName.equals(resolveInfo.activityInfo.name) &&

Actually, instead of removing this code, you should just `continue` if there's a match – we want to remove the item from the share menu as well.

Maybe there's a better way to do this via intent filters, however.
Attachment #8668067 - Flags: review+ → review-
Comment on attachment 8668072 [details] [diff] [review]
Add 'Send to other device' Shareplane to menu

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

r+ w/ nits.

Before this lands, we should also get a summary on how we should handle no account, no other synced devices, & one other synced devices.

::: mobile/android/base/BrowserApp.java
@@ +3395,5 @@
> +                    Intent sendToDeviceIntent = GeckoAppShell.getShareIntent(getContext(), url,
> +                            "text/plain", tab.getDisplayTitle());
> +                    sendToDeviceIntent.setClass(getContext(), ShareDialog.class);
> +                    startActivity(sendToDeviceIntent);
> +                    Telemetry.sendUIEvent(TelemetryContract.Event.SHARE, TelemetryContract.Method.MENU, "sendtab");

We already send a telemetry event for selecting the menu item in this method so this probe seems unnecessary. Good thinking though!
Attachment #8668072 - Flags: review?(michael.l.comella) → review+
The push in comment 44 has a locally fixed build because these patches don't compile through mach.
(In reply to Michael Comella (:mcomella) from comment #34)
> (In reply to Dipti Nirmale [:dnirm] from comment #24)
> > Could you recommend any tool which maps elements on android ui to its id or
> > layout? I generally do it intuitively which may not be the right way. I
> > found [1] on similar lines.
> 
> Hierarchy Viewer would be the only one I know of, but it's not as fast as
> the DOM inspector. I generally read the code.

Thanks for the info Michael.
(In reply to Michael Comella (:mcomella) from comment #35)
> (In reply to Dipti Nirmale [:dnirm] from comment #32)
> > I tried setting the height of the plane to 24dp in [1], [2] & [3] but it did
> > not change the view. Probably Michael could help here. If it is icon design
> > specific requirement, can we have a new bug for this?
> 
> I'm not sure what you mean by "icon design specific requirement" but I'll
> take a look into this and see what I can find.

"icon design specific requirement" - Do we need to resize the icon (for send to device) which means changing the icon specific details? 
I'm not sure if it can be done within time frame since I'm naive to this.
(In reply to Michael Comella (:mcomella) from comment #33)
> (In reply to Dipti Nirmale [:dnirm] from comment #22)
> > > Are these icons necessary? Have you tested on GB? I don't think these items
> > > appear in the top 5 items and thus don't have an icon.
> > 
> > I'm not sure on this. What is the criteria for adding the icons in menu?
> 
> On GB, it's the top five items, I believe. For example, see this menu [1].
> Notably, item 6 on our menu is more, which brings up a list without icons.
> 
> If you don't have a GB device, I can test for you – just NI me.

I don't have GB device. I downloaded 2.3.3 sdk but my AVD manager is not showing GB option for emulator. I'll dig into more to make it work; meanwhile it would be great if you could test it on your device.
Flags: needinfo?(michael.l.comella)
(In reply to Michael Comella (:mcomella) from comment #43)
> Comment on attachment 8668072 [details] [diff] [review]
> Add 'Send to other device' Shareplane to menu
> 
> Review of attachment 8668072 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ w/ nits.
> 
> Before this lands, we should also get a summary on how we should handle no
> account, no other synced devices, & one other synced devices.
> 
> ::: mobile/android/base/BrowserApp.java
> @@ +3395,5 @@
> > +                    Intent sendToDeviceIntent = GeckoAppShell.getShareIntent(getContext(), url,
> > +                            "text/plain", tab.getDisplayTitle());
> > +                    sendToDeviceIntent.setClass(getContext(), ShareDialog.class);
> > +                    startActivity(sendToDeviceIntent);
> > +                    Telemetry.sendUIEvent(TelemetryContract.Event.SHARE, TelemetryContract.Method.MENU, "sendtab");
> 
> We already send a telemetry event for selecting the menu item in this method
> so this probe seems unnecessary. Good thinking though!

For rest of the 'r+ w/ nits' - should I flag "r?" or "r+" (carry forward r+) after uploading patches with corrections?
Attachment #8668067 - Attachment is obsolete: true
Attachment #8670017 - Flags: review?(michael.l.comella)
Attachment #8670018 - Flags: review?(michael.l.comella)
(In reply to Dipti Nirmale [:dnirm] from comment #47)
> "icon design specific requirement" - Do we need to resize the icon (for send
> to device) which means changing the icon specific details? 

We should be able to resize the icon in code via scaleType and ImageView bounds. What do you mean by icon specific details?

> I'm not sure if it can be done within time frame since I'm naive to this.

If we were to change the asset (not preferred), it's generally pretty quick. If Anthony has the assets (e.g. some were from the previous UX designer), he can quickly export from SVG and so the icons are high quality pngs.

(In reply to Dipti Nirmale [:dnirm] from comment #48)
> I don't have GB device. I downloaded 2.3.3 sdk but my AVD manager is not
> showing GB option for emulator.

We actually support running the emulator from the command line now!
  https://wiki.mozilla.org/Mobile/Fennec/Android/Testing#Running_tests_on_the_Android_emulator

> meanwhile it would be great if you could test it on your device.

I'll have a device to test on tomorrow.

(In reply to Dipti Nirmale [:dnirm] from comment #49)
> For rest of the 'r+ w/ nits' - should I flag "r?" or "r+" (carry forward r+)
> after uploading patches with corrections?

You can carry forward the r+ on all patches that are "r+ w/ nits".
Flags: needinfo?(michael.l.comella)
Attachment #8670017 - Flags: review?(michael.l.comella) → review+
Attachment #8670018 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8670017 [details] [diff] [review]
Remove quick share bar and set the action view to Default

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

::: mobile/android/base/menu/GeckoMenuItem.java
@@ -140,5 @@
>      @Override
>      public View getActionView() {
>          if (mActionProvider != null) {
> -            if (getActionEnum() == MenuItem.SHOW_AS_ACTION_IF_ROOM) {
> -                return mActionProvider.onCreateActionView(SECONDARY_ACTION_BAR_HISTORY_SIZE,

You can also remove SECONDARY_ACTION_BAR_HISTORY_SIZE & QUICK_SHARE_ACTION_BAR_HISTORY_SIZE.
Dipti, I applied your patches and this looks great! Thanks for your hard work!

I think we still have to:
  * Test on GB
  * Figure out the expected behavior when the user is not logged in, when there are no other synced devices, and when there is 1 other synced device.

Anthony, here's a build if you want to test it out:
  https://people.mozilla.com/~mcomella/apks/dnirm-1140048_01.apk

I find that the "I'm sending this tab to another device" context is lost... What do you think?

Note: I added a try push in comment 53.

Note2: Since these are attached as individual changesets, it's good to label their order. For example, the first patch would be, "Part 1 – Add 'send to device' and remove quick share from menu resource"
(In reply to Michael Comella (:mcomella) from comment #55)
> I think we still have to:
>   * Test on GB
>   * Figure out the expected behavior when the user is not logged in, when
> there are no other synced devices, and when there is 1 other synced device.
  * Fix remaining nits
  * ? – Resize the share plane in the menu
Looks good! 

Yes, the size of the shareplane would be the last thing to clean up here.

Also, I'm seeing this in the "Share" list on my N6. Anyone else?
Flags: needinfo?(alam)
Wanted to call out the various scenarios/flows here:

Not signed in:
 - Proceed to FxA sign up flow

User is signed in, but has no other connected devices:
 - Display toast with messaging
 - - "Send this tab to another connected device" 

 - At a later point, possibly experiment with using our "in-product promo sheet" UX (bug to-be-filed for this V2 polish)

User is signed in, has 1 other connected device:
 - Just send the tab, confirmation UI will follow (being handled in bug 1147653)

User is signed in, has 2+ other connected device:
 - Show device list, then send the tab to selected device, followed by confirmation UI.

For context, this button sends the current tab to a Firefox Sync connected device. But, we need to display a message for users that don't have another device to send to. 

Essentially, we want the users to understand that they need to sign in on _another_ device to fully benefit from this feature.
Flags: needinfo?(michael.l.comella)
Flags: needinfo?(dnirm.moz)
(In reply to Michael Comella (:mcomella) from comment #36)
> r+ w/ nits.
> 
> ::: mobile/android/base/resources/menu-v11/browser_app_menu.xml
> @@ +42,5 @@
> >      <item android:id="@+id/share"
> >            android:icon="@drawable/ic_menu_share"
> >            android:title="@string/share"
> > +          android:showAsAction="ifRoom|withText">
> > +    </item>
> 
> Use `/>` rather than a separate closing tag.
> 
> ::: mobile/android/base/resources/menu/browser_app_menu.xml
> @@ +36,2 @@
> >      <item android:id="@+id/share"
> > +          android:icon="@drawable/ic_menu_share"
> 
> I don't think this icon is necessary either.

Carry forward r+
Attachment #8668066 - Attachment is obsolete: true
Attachment #8670622 - Flags: review+
(In reply to Michael Comella (:mcomella) from comment #54)
> ::: mobile/android/base/menu/GeckoMenuItem.java
> @@ -140,5 @@
> >      @Override
> >      public View getActionView() {
> >          if (mActionProvider != null) {
> > -            if (getActionEnum() == MenuItem.SHOW_AS_ACTION_IF_ROOM) {
> > -                return mActionProvider.onCreateActionView(SECONDARY_ACTION_BAR_HISTORY_SIZE,
> 
> You can also remove SECONDARY_ACTION_BAR_HISTORY_SIZE &
> QUICK_SHARE_ACTION_BAR_HISTORY_SIZE.

Carry forward r+
Attachment #8670017 - Attachment is obsolete: true
Attachment #8670623 - Flags: review+
Added label.
Carry forward r+
Attachment #8670018 - Attachment is obsolete: true
Attachment #8670624 - Flags: review+
(In reply to Michael Comella (:mcomella) from comment #38)
> r+ w/ nits.
> 
> ::: mobile/android/base/menu/GeckoMenu.java
> @@ +233,5 @@
> > +                    setAdapter(null);
> > +                    addHeaderView((DefaultActionItemBar) mShareActionItemBar);
> > +                    setAdapter(mAdapter);
> > +                }
> > +                    mShareActionItems.put(menuItem, actionView);
> 
> nit: indentation

Carry forward r+
Attachment #8668070 - Attachment is obsolete: true
Attachment #8670625 - Flags: review+
(In reply to Michael Comella (:mcomella) from comment #43)
> r+ w/ nits.
> ::: mobile/android/base/BrowserApp.java
> @@ +3395,5 @@
> > +                    Intent sendToDeviceIntent = GeckoAppShell.getShareIntent(getContext(), url,
> > +                            "text/plain", tab.getDisplayTitle());
> > +                    sendToDeviceIntent.setClass(getContext(), ShareDialog.class);
> > +                    startActivity(sendToDeviceIntent);
> > +                    Telemetry.sendUIEvent(TelemetryContract.Event.SHARE, TelemetryContract.Method.MENU, "sendtab");
> 
> We already send a telemetry event for selecting the menu item in this method
> so this probe seems unnecessary. Good thinking though!

Carry forward r+
Attachment #8668072 - Attachment is obsolete: true
Attachment #8670626 - Flags: review+
(In reply to Michael Comella (:mcomella) from comment #55)
> Dipti, I applied your patches and this looks great! Thanks for your hard
> work!
> 
> Note2: Since these are attached as individual changesets, it's good to label
> their order. For example, the first patch would be, "Part 1 – Add 'send to
> device' and remove quick share from menu resource"

Fixed remaining nits with labeling. Thanks Michael for all the help. I learned through the process.

(In reply to Michael Comella (:mcomella) from comment #56)
> (In reply to Michael Comella (:mcomella) from comment #55)
> > I think we still have to:
> >   * Test on GB
> >   * Figure out the expected behavior when the user is not logged in, when
> > there are no other synced devices, and when there is 1 other synced device.
>   * ? – Resize the share plane in the menu

(In reply to Anthony Lam (:antlam) from comment #58)
> Wanted to call out the various scenarios/flows here:
> 
> Not signed in:
>  - Proceed to FxA sign up flow
> 
> User is signed in, but has no other connected devices:
>  - Display toast with messaging
>  - - "Send this tab to another connected device" 
> 
>  - At a later point, possibly experiment with using our "in-product promo
> sheet" UX (bug to-be-filed for this V2 polish)
> 
> User is signed in, has 1 other connected device:
>  - Just send the tab, confirmation UI will follow (being handled in bug
> 1147653)
> 
> User is signed in, has 2+ other connected device:
>  - Show device list, then send the tab to selected device, followed by
> confirmation UI.
> 
> For context, this button sends the current tab to a Firefox Sync connected
> device. But, we need to display a message for users that don't have another
> device to send to. 
> 
> Essentially, we want the users to understand that they need to sign in on
> _another_ device to fully benefit from this feature.

All this looks challenging and I'd have loved to work on all the pieces. Unfortunately I'm going on a month long vacation as I told Michael last week in IRC conversation. I apologize that due to lack of bandwidth I will not be able to work on the follow-up tasks.
Flags: needinfo?(dnirm.moz)
(In reply to Dipti Nirmale [:dnirm] from comment #64)
> Fixed remaining nits with labeling. Thanks Michael for all the help. I
> learned through the process.

Yay – I'm glad! :)

> (In reply to Michael Comella (:mcomella) from comment #56)
> All this looks challenging and I'd have loved to work on all the pieces.
> Unfortunately I'm going on a month long vacation as I told Michael last week
> in IRC conversation. I apologize that due to lack of bandwidth I will not be
> able to work on the follow-up tasks.

No worries – thanks for all the help you've done thus far! I'll take this across the finish line, but be sure to let us know when you're back and if you're hungry for more!

Enjoy your time off!

Anthony, in order to separate concerns, I'd like to land this as is (provided it works on GB), and take care of the rest in a follow-up. Work for you?
Flags: needinfo?(michael.l.comella) → needinfo?(alam)
Definitely. 

Thanks Dipti!
Flags: needinfo?(alam)
NI self to land & file follow-ups.
Flags: needinfo?(michael.l.comella)
Attached image GB screenshot
I think the Share should be over "Send to other devices" – what do you think, Anthony?
Attachment #8672166 - Flags: feedback?(alam)
https://hg.mozilla.org/integration/fx-team/rev/13a6c5fb2f44fada9cc3dfe832641e0dcb968de7
Bug 1140048 - Add 'send to device' and remove quick share item in toolbar menu. r=mcomella

https://hg.mozilla.org/integration/fx-team/rev/a9d3499bbec176606b4e7c1b8ba2ff91a4c81621
Bug 1140048 - Remove quick share bar and set the action view to Default. r=mcomella

https://hg.mozilla.org/integration/fx-team/rev/35336ca96de213a697277b5c33ba300c553ee1bb
Bug 1140048 - Remove 'Send to other device' menu item from share menu. r=mcomella

https://hg.mozilla.org/integration/fx-team/rev/64e311403f05ef109a5cfbe59babe0b929f4a2b9
Bug 1140048 - Move 'share' button down, replacing quick share view in menu. r=mcomella

https://hg.mozilla.org/integration/fx-team/rev/8b8598ed040e6343da1be02b8bab835362ba1355
Bug 1140048 - Add "Send to other device" Shareplane to top level menu. r=mcomella
Landed – thanks for all your help Dipti! :)

I filed bug 1213486 for the shareplane size and bug 1213490 for the UX when < 2 devices are synced.
Flags: needinfo?(michael.l.comella)
Comment on attachment 8672166 [details]
GB screenshot

wfm
Attachment #8672166 - Flags: feedback?(alam) → feedback+
tldr; backout of 44 inbound.

The shareplane does what we want it to do when there are devices to share to (and the user is logged in) but we couldn't deliver a compelling experience when the user is 1) not signed into a Firefox Account or 2) the Firefox Account they're signed into has no other synced devices. bug 1213490 and bug 1217174 originally covered the implementation.

We decided to develop a helper UI that will walk the user through signing up for a Firefox Account and informing them they may need to sync other devices to send a tab to them. We don't have enough time to get this into 44 so we're going to aim for 45.

btw, Dipti, if you're interested in working on the helper UI, let me know! :)
tracking-fennec: 44+ → 45+
We don't actually need to land bug 1183659 first.
No longer depends on: 1183659
Bug 1140210 prevented a clean backout of this bug so I'm going to back it out too.
Blocks: 1140210
NI self to complete the backout (tree is closed). Update this bug's metadata as well as:
  bug 1140210
  bug 1213490
  bug 1213486
Flags: needinfo?(michael.l.comella)
Verified as fixed in build 44.0a2 2015-11-02;
Devices:
- Motorola Razr (Android 4.4.4);
- Samsung Galaxy R (Android 2.3.4);
- Asus Transformer Pad (Android 4.2.1).
Verified as fixed using:
Device: nexus 9 (android 6.0)
Build: Firefox for Android 45.0a1 (2015-11-11)

"Shareplane" icon is present at the top level menu:
- if the user is not signed in and he taps the icon: the FxA sign up flow is displayed
- if the user is signed in, but has no other connected devices: "Send this tab to another connected device" toast notification is displayed
- if the user is signed in and has 1 other connected device: a dialog with the single device is shown 
- if the user is signed in, has 2+ other connected device: the device list is shown
To double check, Margaret, we're backing this out, and the related work that landed on top, right?
Flags: needinfo?(margaret.leibovic)
(In reply to Michael Comella (:mcomella) from comment #82)
> To double check, Margaret, we're backing this out, and the related work that
> landed on top, right?

Yes, let's back this out, and revisit it in the future when we have a better story around the no-accounts flow. I think this could be more important in the near future, when there's a better send tab experience from desktop.

I would be fine to leave this in Nightly, but since this isn't actually behind a Nightly flag, and keeping it on Nightly would require maintenance work at every merge, it seems easier to just back it out.

That being said, another idea we brought up in the funnel meeting would be not hiding the current shareplane option in the share menu when the user doesn't have an account, and using that to launch the helper UI. While this is less discoverable than a top-level menu, it does increase discoverability of this feature, while avoiding an unused icon in the top-level menu. We could file a bug about this, but I don't want us to invest too much time in this right now.
Flags: needinfo?(margaret.leibovic) → needinfo?(michael.l.comella)
(In reply to :Margaret Leibovic from comment #83)
> That being said, another idea we brought up in the funnel meeting would be
> not hiding the current shareplane option in the share menu when the user
> doesn't have an account, and using that to launch the helper UI. While this
> is less discoverable than a top-level menu, it does increase discoverability
> of this feature, while avoiding an unused icon in the top-level menu. We
> could file a bug about this, but I don't want us to invest too much time in
> this right now.

If we spent the time to develop the helper UI while the feature was still in the share menu, it'd be pretty trivial to reapply these patches to get the whole thing in the top-level menu.

Dipti, I apologize for this getting backed out. As Margaret mentioned we don't have a good experience when the user doesn't have an account or has no other devices to share to in that account (e.g. we can hide the menu item but then it sucks for those users because the menu is strictly worse than it was before). That being said, if you'd like to invest in that experience, we should be able to land this again! :)

I've got the patches ready for backout but I'm waiting on the closed tree.
Thank you Dipti and Mike for all your hard work on this feature. I do think this is something that would be great to revisit in the future, but we just need to invest more in the no-account situation, and I think it would be best to do that without the pressure of needing to ship this in a certain release.

Mike, can we use the existing bugs to work on the helper UI from the regular share menu, or do we need to file a new bug? Can you garden the bug tree here to make sure we have open bugs that reflect the outstanding work? We usually reopen bugs that were backed out, but we could also file new bugs if it makes it easier to follow the work.
Sure, we can re-use the bugs.

The following bugs should be relanded and will force this bug to be rebased on them (hence depends on):
* bug 1210989
* bug 1140210

The following bugs will need to land on top of this when we reland (I don't think order matters):
* bug 1213490
* bug 1213486
* bug 1216307

The helper UI can be implemented in bug 1217174.
No longer blocks: 1217174
Depends on: 1217174
Flags: needinfo?(michael.l.comella)
Status: RESOLVED → REOPENED
tracking-fennec: 45+ → ---
Resolution: FIXED → ---
bug 1216312 should also be relanded once this lands.
Since this is reopened and Dipti is, afaik, not actively working on this, unassigning.

Dipti, let me know if you want to work on this further! We should start by implementing the helper UI (bug 1217174).
Assignee: dnirm.moz → nobody
Anthony, do we still want to do this?
Flags: needinfo?(alam)
I'm gonna loop in Barbara here for her Product expertise.

Barbara, let's revisit this given the landscape of all the other work we're doing around the task continuity stories.
Flags: needinfo?(bbermes)
I think we should still consider this, and it's currently parked as a P3 aha card and hence should not be closed yet, but not worked on (actively) yet either.
Flags: needinfo?(bbermes)
Flags: needinfo?(alam)
May I suggest that rather than convolute the process of sharing between Firefox Account registered devices, we instead use the real estate to add a Sync Now button given the removal of the Synced Tabs page from Firefox Home? Also if some work is to be done to Send To Device, may I suggest adding making it so that we can directly send to devices with Direct Share.

Downgrading priority from P1 to P2

Priority: P1 → P2
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: REOPENED → RESOLVED
Closed: 9 years ago3 years ago
Resolution: --- → INCOMPLETE
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

Created:
Updated:
Size: