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

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect)

Description

[Josh Matthews [:jdm]]

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

Comment 1

[(no longer active)]

Attachment: Fix

Comment 2

[Josh Matthews [:jdm]]

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.

Comment 3

[Josh Matthews [:jdm]]

Looks like just HTTP and nsBaseChannel require changes.

Comment 4

[(no longer active)]

Attachment: Patch (v2)

Comment 5

[Josh Matthews [:jdm]]

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.

Comment 6

[bhavana bajaj [:bajaj]]

Ehsan, can you please confirm if aurora (18.0a2) is affected as well ?

Comment 7

[(no longer active)]

(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 8

[Jason Duell]

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" :)

Comment 9

[Jason Duell]

ahem--make that "nsBaseChannel::Redirect may be dead code"  :)

Comment 10

[(no longer active)]

(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 11

[(no longer active)]

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.

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/007e45a1f6c9

Comment 13

[(no longer active)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/faac541d3a9d

Comment 14

[Paul Silaghi, QA [:pauly]]

No suggestions stored in the disk cache. Verified fixed FF 18b3, Aurora 19.0a2 (2012-12-11).