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
Bug 1367921 - add 'sign in to sync' action to send tab/page/link to device if sync is not configured,
59 bytes, text/x-review-board-request
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]
Priority: -- → P2
Whiteboard: [photon-structure] [triage] → [photon-structure]
Priority: P2 → P3
Whiteboard: [photon-structure] → [reserve-photon-structure]
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?
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
(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? :-)
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.
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 firstname.lastname@example.org: 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 email@example.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 firstname.lastname@example.org: 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
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]
You need to log in before you can comment on or make changes to this bug.