Syncing… in devices list should say Syncing Tabs…

RESOLVED FIXED in Firefox 57

Status

()

Firefox
Sync
P2
normal
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: rfeeley, Assigned: Steve Armand, Mentored)

Tracking

({good-first-bug})

55 Branch
Firefox 57
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

3 months ago
"Sync Now" in Synced Tabs does not perform a full Sync but rather just refreshes the list of devices. It should therefore just say "Refresh Devices".

While refreshing it should say "Refreshing…" (and not "Syncing…")

The tooltip should not be prefixed with "Last sync:" but rather "Updated:"
(Reporter)

Comment 1

3 months ago
A correction to the above… After learning that this feature also syncs the currently opened tabs, we can take a much less radical approach.

The only change required is to change the mid-sync state of "Syncing…" to "Syncing Tabs…"

That provides just enough context for all users to understand that a full sync is not happening.
Summary: Sync Now in devices list should say Refresh Devices → Syncing… in devices list should say Syncing Tabs…
Mentor: eoger@fastmail.com
Keywords: good-first-bug
Priority: -- → P2

Comment 2

3 months ago
Hi, I am a beginner. Can I take it up? Also, it would be great if you could provide me with some starter points!
Flags: needinfo?(rfeeley)
(Assignee)

Comment 3

3 months ago
Created attachment 8901615 [details] [diff] [review]
Change the mid-sync state of "Syncing…" to "Syncing Tabs…"
(Assignee)

Comment 4

3 months ago
Hi, I'm new as well. I'm submitting the attached patch as my first. Please advise on next steps!
(Assignee)

Updated

3 months ago
Attachment #8901615 - Flags: review?(eoger)

Comment 5

3 months ago
Comment on attachment 8901615 [details] [diff] [review]
Change the mid-sync state of "Syncing…" to "Syncing Tabs…"

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

Thanks for the patch, but due to how Firefox is localized, the string ID also needs to be changed. ie, sync.properties probably needs to change to something like:

syncingtabs.label = Syncing Tabs…

and all references in the code to |syncing2.label| need to change to |syncingtabs.label|

Could you please make these changes and re-upload the patch for review. Ideally, you can try to follow the dev guide and push the patch to mozreview. If you change the commit message to |Bug 1390644 - Change the mid-sync state of "Syncing…" to "Syncing Tabs…". r?eoger", then when you push to mozreview it will automatically ask Edouard for review and will simplify the process of pushing it once it is accepted. Let us know if you need help with any of that!
Attachment #8901615 - Flags: review?(eoger)

Updated

3 months ago
Assignee: nobody → stevea1
Flags: needinfo?(rfeeley)
Comment hidden (mozreview-request)
(Assignee)

Comment 7

3 months ago
Ok, I've made the recommended changes and pushed to mozreview.

Comment 8

3 months ago
mozreview-review
Comment on attachment 8902217 [details]
Bug 1390644 - Change the mid-sync state of "Syncing…" to "Syncing Tabs…".

https://reviewboard.mozilla.org/r/173696/#review179114

Works great, thank you for your patch!
Attachment #8902217 - Flags: review?(eoger) → review+

Comment 9

3 months ago
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3a87086e6552
Change the mid-sync state of "Syncing…" to "Syncing Tabs…". r=eoger

Comment 10

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3a87086e6552
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
You need to log in before you can comment on or make changes to this bug.