If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Fix the wrong comment in nsHttpChunkedDecoder

RESOLVED FIXED in Firefox 54

Status

()

Core
Networking
RESOLVED FIXED
8 months ago
7 months ago

People

(Reporter: kershaw, Assigned: njfox)

Tracking

({good-first-bug})

Trunk
mozilla54
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

(Whiteboard: [necko-backlog])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 months ago
The chunked transfer coding [1] is actually coming from RFC2616 [2], not RFC2617.

[1] http://searchfox.org/mozilla-central/rev/f5077ad52f8b90183e73038869f6140f0afbf427/netwerk/protocol/http/nsHttpChunkedDecoder.cpp#30
[2] https://tools.ietf.org/html/rfc2616#section-3.6.1
(Reporter)

Updated

8 months ago
Keywords: good-first-bug
Whiteboard: [necko-backlog]
(Assignee)

Comment 1

7 months ago
Submitted changeset for review.
(Assignee)

Comment 2

7 months ago
Created attachment 8842509 [details] [diff] [review]
Bug 1337303 - Corrected RFC number cited in comments

Reviewed by mcmanus; I had attached this to the wrong bug before.
Attachment #8842509 - Flags: review+
Attachment #8842509 - Flags: checkin+
(Assignee)

Updated

7 months ago
Attachment #8842509 - Flags: checkin+
Comment hidden (mozreview-request)
Comment on attachment 8842509 [details] [diff] [review]
Bug 1337303 - Corrected RFC number cited in comments

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

somehow you have created a file called commit-message-f58fb you don't want a new file in the repo like that.
Attachment #8842509 - Flags: review+ → review-

Comment 5

7 months ago
mozreview-review
Comment on attachment 8842574 [details]
Bug 1337303 - Corrected RFC number cited in comments

https://reviewboard.mozilla.org/r/116358/#review118004

duplicate review request
Attachment #8842574 - Flags: review?(mcmanus)
(Assignee)

Comment 6

7 months ago
Created attachment 8842622 [details] [diff] [review]
Started over on this commit to fix various issues with the patch
Attachment #8842509 - Attachment is obsolete: true
Attachment #8842574 - Attachment is obsolete: true
Attachment #8842622 - Flags: review+
(Assignee)

Updated

7 months ago
Keywords: checkin-needed
(Assignee)

Comment 7

7 months ago
Latest attachment should be good to go. This is a comment-only change, so this patch should not need a try run to prove it's ok. Thanks for your patience while I learned the workflow for contributing.

Comment 8

7 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/74b41dce2486
Corrected RFC number cited in comments. r=mcmanus
Keywords: checkin-needed
Assignee: nobody → nick
Status: NEW → ASSIGNED

Comment 9

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/74b41dce2486
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
nick, thanks for the patch and this all worked out ok so we don't need to take any action.

But in comment 4 I revoked your r+ and changed it to a r-.. that meant you didn't have a r+ to self-assign and then mark checkin-needed when you fixed the patch. (you fixed it correctly, so I would have then given you the r+.. but I changed it to r- because I wanted to confirm it as you're a new contributor.. I should have made that more clear to you when I did it - its certainly not common.) I'm sure it was just done out of confusion (afterall I shouldn't have r+'d the patch at all if I was unhappy - I just realized afterwards what I was seeing in there with the extra file).

no worries - just clarifying the workflow because the tools (intentionally) let people override this stuff.
(Assignee)

Comment 11

7 months ago
Thanks Patrick, and sorry about that. It is definitely out of confusion :) I am reaching out to people on IRC to figure out what I'm doing wrong.
You need to log in before you can comment on or make changes to this bug.