298705 - live bookmarks should only refresh when browser is active

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, enhancement)

Description

[Martin N.]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

Lots of websites that offer RSS feeds are wasting a lot of traffic now that live
bookmarks are so convenient.  Additionally, lots of users leave their web
browsers open - for e.g. - overnight.  This results in lots of wasted updates.

  There should be an option for live bookmarks, preferably on by default, that
prevents them from being updated when the browser isn't active - or perhaps a
backoff algorithm could be implemented based on activity (e.g. update less
frequently the less the system has been used).  I don't know what kind of hooks
Firefox/Mozilla does or doesn't have with the host OS - but it would be
preferable to query the host instead of just Firefox for the idle time.  In any
case, it shouldn't take very long to update live bookmarks, once there is
activity in the Firefox window the browser could check if the live bookmarks
have been updated within their normal time limit, and do so if not.

  This would result in many fetches at once, however, for users with lots of
live bookmarks.  I don't pretend to have a solution for such a thing, but
chances are they're big contributors to the problems in any case.

  I just see this as another good way for Firefox to "do the right thing" and
lead the way before it becomes standard to blindly grab RSS feeds because "oh
they're so small".

  I also don't know whether Firefox checks if the feeds have changed before
fetching the whole thing.  Whether that would be preferable would depend on the
size of the feed, and the frequency of it updating, but I expect in the majority
of cases it would be preferable [ it's already probably implemented, however.  I
should test that... :) ]

Reproducible: Always

Steps to Reproduce:

Comment 1

[zug_treno]

Related to bug 294269?

Comment 2

[Martin N.]

No, that's an attempt not to try to update when you're offline.  This is rather
the opposite - to not update when the machine is online, but the user is not
present.

I doubt it's anything that many users would push for, but it's certainly
something that would be a good default to have in place.  Essentially, (by
default) Firefox would not update live bookmarks when a user is not actively
using their machine.

I could see it being related to, but not the same as, 261076 - as both have the
intention of basically being nicer to servers, and be able to control usage better.

[ again, as mentioned, this assumes that the core can actually track this
information across OS targets - otherwise this is likely extension material. ]

Comment 3

[Stephanie Daugherty [:sdaugherty]]

I would concur with comment #2 that this is probably extension material. As far as I know, Firefox doesn't track whether the user is active or not - and generally doesn't need to. Suggesting WONTFIX.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

I don't think this should be WONTFIX. Once the work in bug 343416 lands, we should be able to do this on Linux, and it should be fairly easy to implement that interface for other platforms.

Comment 5

[Shawn Wilsher :sdwilsh]

(In reply to comment #4)
> I don't think this should be WONTFIX. Once the work in bug 343416 lands, we
> should be able to do this on Linux, and it should be fairly easy to implement
> that interface for other platforms.

Windows and OSX are now implemented as well.

Comment 6

[:Gijs (he/him)]

I can poke this for a bit, I think.

Comment 7

[:Gijs (he/him)]

Attachment: Untested patch

I haven't tested this patch properly. I tried for 3 hours, in which I found out the following:
- rebuilding mac os app bundles sucks.
- debugging js components sucks more.
- nsLivemarkService.js does get copied to the components bits in your dist/bin and app bundle, even though it's completely and utterly unused without adding the right build flags.
- nsLivemarkService.js has hard tabs in weird places, and spaces everywhere else (huh?)
- places builds, but bookmarks aren't enabled by default (see above)



Either way, I'm pretty sure this patch should work, but this thing has exhausted any and all patience I had with it. Poke me again when this has r+ and/or places is actually usable (or is clear about not being usable) if you're unable to test it yourself.

Oh, and I picked half an hour for the default idle time. I don't think there should be a UI pref, so I didn't make one. Anyone who wants to can engage in idealistic debate about both those decisions - I don't think it matters much, seeing as the default delay between checking live bookmarks is an hour (and every 10 minutes if they were unparseable), and the current system will notice within 15 seconds that the user is no longer idle.

Comment 8

[:Gijs (he/him)]

Comment on attachment 261001 [details] [diff] [review]
Untested patch

*kicks bugzilla*

Comment 9

[:Gijs (he/him)]

Attachment: Better but still untested patch

Mossop pointed out I should check whether the contract ID is present for the sake of those running non-win32 non-mac non-gtk2. Oops.

Comment 10

[Asaf Romano (gone)]

Comment on attachment 261002 [details] [diff] [review]
Better but still untested patch

>Index: toolkit/components/places/src/nsLivemarkService.js
>===================================================================


>+      // Check the user idle time. If the user isn't using their computer, don't
>+      // bother updating - save the internet some bandwidth. If we can't
>+      // get the idle time, assume the user isn't idle.
>+      var idleTime = 0;
>+      try {
>+        idleTime = this._idleService.idleTime;
>+      } catch (ex) { /* We don't care */ }
>+

Please make it so we call GetIdleTime only after the "cheaper" checks.

r=mano otherwise.

Comment 11

[:Gijs (he/him)]

Attachment: Patch with review comments addressed

Patch with review comments addressed. This works, as far as I can tell (though I changed the constant values when testing, as I am way too lazy to wait for an hour).

Comment 12

[Asaf Romano (gone)]

Comment on attachment 278483 [details] [diff] [review]
Patch with review comments addressed

r=mano

Comment 13

[:Gijs (he/him)]

Checking in mozilla/toolkit/components/places/src/nsLivemarkService.js;
/cvsroot/mozilla/toolkit/components/places/src/nsLivemarkService.js,v  <--  nsLivemarkService.js
new revision: 1.24; previous revision: 1.23
done

Comment 14

[Tracy Walker [:tracy]]

reporter, please verify this fix.  thank you.

Comment 15

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv