356463 - add X-Moz: livebookmark to http request when doing live bookmark refreshes

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Mike Beltzner [:beltzner, not reading bugmail]]

Feedburner has asked us to help them differentiate between normal HTTP traffic and
LiveBookmark refresh requests on their site. For prefetch and microsummaries, we add an extra header to the http request to allow sites to filter (X-Moz: prefetch and X-Moz: microsummary) We should add a similar header, X-Moz: livebookmark to the http requests for live bookmark refresh requests.

The URL field points to a place where it could, according to mconnor, be added pretty easily to nsBookmarksFeedHandler

Comment 1

[Robert Sayre]

should fix this in bug 353434 as well.

Comment 2

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

Attachment: patch

This also removes some code which, as far as I can tell, does nothing.

Comment 3

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

mozilla/browser/components/bookmarks/src/nsBookmarksFeedHandler.cpp 	1.18

Comment 4

[Robert Sayre]

Does this meet branch criteria?

Comment 5

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

(In reply to comment #4)
> Does this meet branch criteria?

I'm not entirely sure what you're asking, since I don't think "[1.8] branch criteria" have been clearly defined yet (they usually vary for each minor release), but I think it's a low-risk and high-value fix, which is why I asked for approval. Beltzner's nomination as a blocker also led me to believe there was a strong desire to get this in on the branch so that FeedBurner (and others) could make use of it as soon as possible. Is there something about the patch that you're particularly concerned about?

Comment 6

[Robert Sayre]

(In reply to comment #5)
> 
> I don't think "[1.8] branch criteria" have been clearly defined yet 

Agree.

> I think it's a low-risk and high-value fix

I agree that it's low-risk. Disagree that it's high-value. Taking a lot of low-risk, low-value patches is not a good recipe for branch stability. Why do we (or FeedBurner) want this?

Comment 7

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

(In reply to comment #6)
> > I think it's a low-risk and high-value fix
> 
> I agree that it's low-risk. Disagree that it's high-value. Taking a lot of
> low-risk, low-value patches is not a good recipe for branch stability. Why do
> we (or FeedBurner) want this?

I suppose you're right that this patch's high-value hasn't yet been clearly demonstrated (at least not in this bug). All of what I know about why FeedBurner wants it is in comment 0, so I can't answer specific questions about their desires. That being said, providing a way to easily identify live bookmark requests may be valuable to us for the sole reason that it would enable feed providers to get better metrics about Firefox usage on their site.

Comment 8

[Jay Patel [:jay]]

Comment on attachment 242144 [details] [diff] [review]
patch

Given the low risk of this bug, rather than wait until next time, let's just get it in.

Approved for 1.8.1 branch, a=jay for drivers.

Comment 9

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

mozilla/browser/components/bookmarks/src/nsBookmarksFeedHandler.cpp 	1.8.2.7

Comment 10

[Chris Cooper [:coop] (he/him)]

WFM, tested with deprec4ted.net:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.1pre) Gecko/20061201 BonEcho/2.0.0.1pre