Closed Bug 356463 Opened 13 years ago Closed 13 years ago

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

Categories

(Firefox :: Bookmarks & History, defect)

2.0 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 2

People

(Reporter: beltzner, Assigned: Gavin)

References

()

Details

(Keywords: verified1.8.1.1)

Attachments

(1 file)

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
Flags: blocking1.8.1.1?
should fix this in bug 353434 as well.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 2
Attached patch patchSplinter Review
This also removes some code which, as far as I can tell, does nothing.
Attachment #242144 - Flags: superreview?
Attachment #242144 - Flags: review?
Attachment #242144 - Flags: superreview?(darin)
Attachment #242144 - Flags: superreview?
Attachment #242144 - Flags: review?(vladimir)
Attachment #242144 - Flags: review?
Attachment #242144 - Flags: superreview?(darin) → superreview?(cbiesinger)
Attachment #242144 - Flags: superreview?(cbiesinger) → superreview+
Attachment #242144 - Flags: approval1.8.1.1?
mozilla/browser/components/bookmarks/src/nsBookmarksFeedHandler.cpp 	1.18
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Does this meet branch criteria?
(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?
(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?
(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 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.
Attachment #242144 - Flags: approval1.8.1.1? → approval1.8.1.1+
Flags: blocking1.8.1.1? → blocking1.8.1.1-
mozilla/browser/components/bookmarks/src/nsBookmarksFeedHandler.cpp 	1.8.2.7
Keywords: fixed1.8.1.1
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.