Closed Bug 739820 Opened 10 years ago Closed 7 years ago

Sync about:newtab pinned items

Categories

(Firefox :: Sync, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
relnote-firefox --- 36+

People

(Reporter: darktrojan, Assigned: gcp)

References

Details

(Keywords: relnote, Whiteboard: [sync-engine-addition])

Attachments

(1 file)

I should be able to switch to Firefox on a different machine and still have the same items in about:newtab.
Component: Tabbed Browser → Firefox Sync: Backend
Product: Firefox → Mozilla Services
QA Contact: tabbed.browser → sync-backend
No longer blocks: 455553
Depends on: 455553
Whiteboard: [sync-engine-addition]
Duplicate of this bug: 763263
After bug 791447 newtab pages data is stored in prefs, so would be really easy to sync them across desktop versions (at least).
Depends on: 791447
Someone asked me about this. After looking for 2 minutes, it looks all we need is to add

services.sync.prefs.sync.browser.newtabpage.pinned = true

to the default prefs?
Flags: needinfo?(rnewman)
(In reply to Gian-Carlo Pascutto [:gcp] from comment #3)
> Someone asked me about this. After looking for 2 minutes, it looks all we
> need is to add
> 
> services.sync.prefs.sync.browser.newtabpage.pinned = true
> 
> to the default prefs?

Something like that is correct; the issue will be merging two different sets of pinned about:newtab items.  I don't know the format of browser.newtabpage.pinned, and I don't know if the prefs merging code has support for per-pref custom logic.
It has no prefs merging at all, and syncing isn't per-pref.

With that pref set to true: whenever browser.newtabpage.pinned -- or any other syncable pref -- changes, a copy of all syncable prefs (not just that one) will be pushed to the sync server, and every other client will download that bundle and overwrite all of their local prefs with those.

Whichever client sets that flag to `true` (or otherwise is prompted to sync prefs) first will 'win'; every other device will have that client's pinned sites.

Prefs sync, like the rest of Sync, is not very sophisticated.
Flags: needinfo?(rnewman)
This seems to be acceptable behavior to me, but maybe an issue is that having Sync "Preferences" imply "Sync New Tabs Page" could be confusing. Given that syncing preferences really makes the browser act exactly alike everywhere, I'm going to go out and guess that people who turn that on *would* want their pinned tabs synced.

I'm going to put uiwanted to get a final call here.
Keywords: uiwanted
IMHO synchronization for "dials" is absolutely a must... What people want is generally what you can find in the awesome Opera speed dial: customizable page with custom number of dials, fluid layout, possibility of adding own dial from a handy GUI (instead of drag something from other places for example, a very hidden feature), sync between devices
See comment 5 & 6.
Flags: needinfo?(madhava)
Sure, let's do it.
Flags: needinfo?(madhava)
Attachment #8495796 - Flags: review?(rnewman)
Comment on attachment 8495796 [details] [diff] [review]
Sync about:newtab pinned items

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

This looks incrementally fine to me, but I'm not sure it'll do the right thing if syncing across different Firefox versions -- consider browser.newtab.enhanced, browser.newtabpage.enabled, and friends.

Do we need to sync other prefs to avoid surprising behavior here? Flagging Mardak and Tim for their input, 'cos once this genie escapes it ain't going back in the bottle.
Attachment #8495796 - Flags: review?(ttaubert)
Attachment #8495796 - Flags: review?(rnewman)
Attachment #8495796 - Flags: review+
Attachment #8495796 - Flags: feedback?(edilee)
From trying out the solution gcp suggested I encountered that just syncing "services.sync.prefs.sync.browser.newtabpage.pinned" is not enough, you'd also need to sync "services.sync.prefs.sync.browser.newtabpage.columns" and "services.sync.prefs.sync.browser.newtabpage.rows" to get the same newtabpage on every synced browser. I guess this can be added to the patch without any additional problems further down the line.
Comment on attachment 8495796 [details] [diff] [review]
Sync about:newtab pinned items

(In reply to Richard Newman [:rnewman] from comment #11)
> This looks incrementally fine to me, but I'm not sure it'll do the right
> thing if syncing across different Firefox versions -- consider
> browser.newtab.enhanced, browser.newtabpage.enabled, and friends.
It's no different than if the user uses Nightly then Release then another version. The value getting synced is an array of objects containing enough link data to have it show the right title and url of the desired position index.

I'll note that one potential confusion is that syncing is more likely to run into situations where different size new tab pages are shown. This means with the new fixed-tile-size layout, one device might show 5 columns while another can only fit 3, so a "pinned to index 4" might show on the first row or second row based on the window size. (But this is no different from the current behavior of the user resizing the window to be bigger/smaller.)
Attachment #8495796 - Flags: feedback?(edilee) → feedback+
Not sure about .rows and .columns as those are screensize-dependent, and screen sizes won't necessarily match. .enabled and .enhanced seem like we'd want to add them, though.
Duplicate of this bug: 1054656
Comment on attachment 8495796 [details] [diff] [review]
Sync about:newtab pinned items

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

I'm not convinced that syncing newtab tiles is the right thing to do. Assuming I have separate machines at work and home then it seems like I would access very different websites on those. OTOH when setting up Sync between those two machines we might have some tiles pop up due to their frecency values but that wouldn't override pinned tiles at least. If at home I pinned reddit I probably wouldn't like to override that with an intranet page from work or vice versa.
Attachment #8495796 - Flags: review?(ttaubert)
Syncing preferences also changes stuff like the default homepage. So I don't think that's a sufficient argument against - especially given that you're arguing for split use (rather than the user using Firefox on different devices) i.e. where the user has different use patterns and never wants to visit the same sites as home, but where the user has still elected to use Sync and sync prefs. I think it's more sensible to disable pref syncing on the work machine then, but still allow the home tiles to propagate to all other devices.

I see a few "yay" and one "mmmaybenay", so I'll let the genie out. 

I think we'll need extra work to make this work on Firefox for Android and Firefox OS, will file follow-ups for that.
(In reply to Gian-Carlo Pascutto [:gcp] from comment #17)
> I think it's more sensible to disable pref syncing on
> the work machine then
> but still allow the home tiles to propagate to all
> other devices.

That's not how Sync works. The same things sync between all devices. If you're signed in to Sync on your work machine, and you turn on prefs sync at home, it'll turn on at work.

It's worth noting that because we use prefs to decide which prefs sync, and those prefs themselves sync, it would be straightforward to allow the user to decide which aspects of Firefox they want shared between devices.

It seems increasingly apparent that one size does not fit all.


> I think we'll need extra work to make this work on Firefox for Android and
> Firefox OS, will file follow-ups for that.

Fennec doesn't sync prefs at all -- nor are we likely to build that, given that most features we care about are a mishmash of per-profile and per-app SharedPreferences, not Gecko prefs. Non-trivial to build, low-value, very hard to get right.

Furthermore, synced prefs are per-app (desktop-to-desktop, XUL Fennec to XUL Fennec, Seamonkey to Seamonkey), and Sync hasn't been implemented for Firefox OS, so you can probably save yourself the effort of filing those bugs!
>you turn on prefs sync at home, it'll turn on at work.

Oh yikes, did not see that coming. So if you enable Prefs sync, the other sync settings are no longer under your (local) control. I think that supports the fact that you don't want to turn this on in Firefox profiles that have dissimilar use.

As for mobile Firefoxen, here we see that although this sits in prefs right now, in terms of behavior it's really closer to bookmarks and history - especially since both our mobile platforms have the concept of a homescreen with a dial. But if you don't think it's going to be worth the effort any time soon then indeed I'll save myself the trouble...
After discussing with rnewman, we might want to test the following case:

1) 2 updated Firefoxes that syncs these prefs, enable pref sync on one, let prefs sync, let pinned tabs sync
2) "Update/downgrade" both Firefoxes to a version that no longer syncs pinned tabs
3) Check if pinned tabs are still being synced.

The potential issue is that the synced prefs might be seen as user-set prefs on the machines where they are set by sync, which would mean that there's no way to "undo" this change without having all affected users edit their about:config.

On the positive side, we just went Firefox 35->36 so there's 6 weeks of testing where only Nightly users would get screwed.
I ran through the migration scenarios today and if Firefox instances don't update in lock-step and sync in-between, you can indeed have the scenario rnewman identified, where the Firefox that doesn't know about the default pref change will receive it and consider it as a user-set pref, store that in Sync, and when downgrading/reverting this patch, Sync will re-set it because it thinks the pref was user-set. Whether you hit this or not depends on the exact order of sync/upgrade/downgrade steps.

That said, worst case this can be reverted by renaming the relevant pref, at the cost of losing pinned tabs.
Keywords: uiwantedrelnote
https://hg.mozilla.org/mozilla-central/rev/d6e705456c79
Assignee: nobody → gpascutto
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Added to the release notes with "Pinned tiles on the new tab page can be synced". Only for desktop (let me know if that changed).
Thanks GCP
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.