Closed Bug 800952 Opened 8 years ago Closed 8 years ago

Visiting http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/ in PB mode stores a favicon in the disk cache

Categories

(Core :: Networking: HTTP, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla19
Tracking Status
firefox18 + verified
firefox19 + verified

People

(Reporter: jdm, Assigned: ehsan)

References

Details

(Whiteboard: [testday-20121012])

Attachments

(1 file, 1 obsolete file)

STR:
1. clear disk cache
2. enter PB mode
3. visit http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
4. see http://www.mozilla.org/media/img/favicon.png in the disk cache
Attached patch Fix (obsolete) — Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #670902 - Flags: review?(josh)
Component: General → Networking: HTTP
Attachment #670902 - Flags: review?(josh) → review+
Comment on attachment 670902 [details] [diff] [review]
Fix

My mistake, we need to do this for at least nsBaseChannel as well. I'm looking into the other ones, too.
Attachment #670902 - Flags: review+ → review-
Looks like just HTTP and nsBaseChannel require changes.
Attached patch Patch (v2)Splinter Review
Attachment #670902 - Attachment is obsolete: true
Attachment #670957 - Flags: review?(josh)
Comment on attachment 670957 [details] [diff] [review]
Patch (v2)

I'm going to be safe here and delegate to Jason, since redirects are slippery beasts.
Attachment #670957 - Flags: review?(josh)
Attachment #670957 - Flags: review?(jduell.mcbugs)
Attachment #670957 - Flags: feedback+
Whiteboard: [testday-20121012]
Ehsan, can you please confirm if aurora (18.0a2) is affected as well ?
(In reply to bhavana bajaj [:bajaj] from comment #6)
> Ehsan, can you please confirm if aurora (18.0a2) is affected as well ?

It is.  The status-firefox18:affected flag reflects that.
Comment on attachment 670957 [details] [diff] [review]
Patch (v2)

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

::: netwerk/base/src/nsBaseChannel.cpp
@@ +77,5 @@
>    newChannel->SetNotificationCallbacks(mCallbacks);
>    newChannel->SetLoadFlags(mLoadFlags | LOAD_REPLACE);
>  
> +  // Try to preserve the privacy bit if it has been overridden
> +  if (mPrivateBrowsingOverriden) {

AFAICT nsBaseChannel may be dead code--I can't seem to get anything to hit it in gdb at least--but you're right that if we do hit it we need this.

::: netwerk/protocol/http/HttpBaseChannel.cpp
@@ +1542,5 @@
>    newChannel->SetNotificationCallbacks(mCallbacks);
>    newChannel->SetLoadFlags(newLoadFlags);
>  
> +  // Try to preserve the privacy bit if it has been overridden
> +  if (mPrivateBrowsingOverriden) {

I guess this typo is part of the code base now, a la Http referer. Don't want to mess up hg history to fix it.  Correct pronunciation rhymes with "Biden" :)
Attachment #670957 - Flags: review?(jduell.mcbugs) → review+
ahem--make that "nsBaseChannel::Redirect may be dead code"  :)
(In reply to Jason Duell (:jduell) from comment #9)
> ahem--make that "nsBaseChannel::Redirect may be dead code"  :)

Wouldn't it be just lovely if nsBaseChannel was dead code?  ;-)

https://hg.mozilla.org/integration/mozilla-inbound/rev/007e45a1f6c9
Comment on attachment 670957 [details] [diff] [review]
Patch (v2)

I think this is reasonably low-risk, and we definitely need to take this for Firefox 18.
Attachment #670957 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/007e45a1f6c9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Attachment #670957 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
No suggestions stored in the disk cache. Verified fixed FF 18b3, Aurora 19.0a2 (2012-12-11).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.