Closed Bug 1367921 Opened 3 years ago Closed 2 years ago

Change label text in Page Action Menu > Send to Device when user isn't signed in and update sync icon

Categories

(Firefox :: Menus, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 57
Iteration:
57.2 - Aug 29
Tracking Status
firefox57 --- fixed

People

(Reporter: abenson, Assigned: Gijs)

Details

(Whiteboard: [reserve-photon-structure][photon-l10n-risk])

Attachments

(1 file)

Currently the label reads: Firefox Account (with an old sync icon)

Update to read: Sign in to Sync (with new icon shown in spec)

This change is to maintain parity with messaging used elsewhere. 


Updated spec: https://mozilla.invisionapp.com/share/KZBRDI1EU#/234257205_Action_Menu_-_Send_To_Device
Whiteboard: [photon-structure] → [photon-structure] [triage]
Flags: qe-verify-
Priority: -- → P2
Whiteboard: [photon-structure] [triage] → [photon-structure]
Priority: P2 → P3
Whiteboard: [photon-structure] → [reserve-photon-structure]
Priority: P3 → P4
Whiteboard: [reserve-photon-structure] → [reserve-photon-structure][photon-l10n-risk]
When trying to reproduce this, I see "Not connected to sync", which is greyed out. I assume this is the thing that needs updating, even if it's not the "Firefox Account" comment #0 mentions.

The subview currently has a "Learn About Sending Tabs..." entry as well - do you want to keep that or should that be removed in the not-signed-in case?
Flags: needinfo?(abenson)
Also, it seems this UI is shared with the UI of the context menu on both the tab and the page. I assume we need to make the same change there...
Component: Menus → Sync
This appears to have changed a bit since I filed the bug. The current state actually looks pretty good but we need a sign in action for those that are already account holders.

Add a line above the learn more row so the menu is ordered like this:

--------
Not connected to sync
--------
Sign in to Sync (links to => about:preferences?entrypoint=pageactionmenu#sync)
Learn About Sending Tabs...
Component: Sync → Menus
Flags: needinfo?(abenson)
(In reply to :Gijs from comment #2)
> Also, it seems this UI is shared with the UI of the context menu on both the
> tab and the page. I assume we need to make the same change there...

Uh, yeah definitely. Not having a sign in action is pretty user hostile IMO.
As the action requires further confirmation, I believe it should it be:

Sign in to Sync…
(In reply to Ryan Feeley [:rfeeley] from comment #5)
> As the action requires further confirmation, I believe it should it be:
> 
> Sign in to Sync…

I made this change, though note that the hamburger panel has always done without the ellipses. If that needs updating too, please file a separate bug.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 57.2 - Aug 29
Priority: P4 → P1
Good call Gijs. I forget about that. We did make an exception for Sync.
(In reply to Ryan Feeley [:rfeeley] from comment #8)
> Good call Gijs. I forget about that. We did make an exception for Sync.

Wait, so does that mean I should remove the ellipsis again from this item? :-)
Flags: needinfo?(rfeeley)
From the patch I'm reading, this will also add a "Sign in to sync" submenu to the context menu, is that ok with you Ryan?
It should have the … in this context. Sorry for the ambiguity.

And yes, thanks for taking care of "Sign in to Sync…" in context menu.
Flags: needinfo?(rfeeley)
Comment on attachment 8900780 [details]
Bug 1367921 - add 'sign in to sync' action to send tab/page/link to device if sync is not configured,

https://reviewboard.mozilla.org/r/172226/#review177604
Attachment #8900780 - Flags: review?(eoger) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/be0fe9a8d424
add 'sign in to sync' action to send tab/page/link to device if sync is not configured, r=eoger
Backout by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8f2f6bceb5cc
Backed out changeset be0fe9a8d424 for browser_contextmenu_sendpage.js and browser_page_action_menu.js browser-chrome failures on a CLOSED TREE.
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a3b50a51383b
add 'sign in to sync' action to send tab/page/link to device if sync is not configured, r=eoger
https://hg.mozilla.org/mozilla-central/rev/a3b50a51383b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I have reproduced this bug with Nightly 55.0a1(2017-05-25) on Windows 8.1 ! 

This bug's fix is Verified with latest Nightly 57.0a1!

Build   ID    20170828100127
User Agent    Mozilla/5.0 (Windows NT 6.3; WOW64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [bugday-20170823]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.