294524 - Reload buttons doesn't work after sanitizing

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[José Jeria]

Build id: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050517 Firefox/1.0+

The reload button is active but doesn't react to clicks after sanitizing Firefox

Steps to reproduce:
1) Launch FF
2) Load ANY page
3) Click on ANY link
4) Click the BACK button
5) Santize FF (only browsing history is enough)
6) Click on the RELOAD button, nothing will happen

Comment 1

[Yani (fr)]

(In reply to comment #0):
Same bug for me using.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050517
Firefox/1.0+  (bangbang023)
Windows XP SP2.

Comment 2

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050517
Firefox/1.0+ ID:2005051707

WFM
but i have History=0 days set

Comment 3

[Bryan]

> Steps to reproduce:
> 1) Launch FF
> 2) Load ANY page
> 3) Click on ANY link
> 4) Click the BACK button
> 5) Santize FF (only browsing history is enough)
> 6) Click on the RELOAD button, nothing will happen

CONFIRMED!

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050517
Firefox/1.0+

Comment 4

[Peter van der Woude [:Peter6]]

(In reply to comment #2)
> Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050517
> Firefox/1.0+ ID:2005051707
> 
> WFM
> but i have History=0 days set
Ignore this comment, my sanitize was malfunctioning (not working at all) again.

Comment 5

[Bryan]

Just an update:

If you follow these steps EXACTLY and have history set to something greater then
0 and have sanitize set to clear your browsing history then you WILL experience
this issue:

1) Launch FF
2) Load ANY page
3) Click on ANY link
4) Click the BACK button
5) Santize FF (only browsing history is enough to trigger this bug)
6) Click on the RELOAD button, nothing will happen

Comment 6

[Teune van Steeg]

Attachment: this will fix the reload of the current page after sanitizing, if there is a forward history

Currently, by purging the session history when a forward history is present,
the current displayed history entry is removed from the history. This causes
the reload of the page to fail.

This patch resolves this by only purging the history upto the current index.

With this patch there is a downside however: if there is a forward history, the
forward history will not be deleted. So when the tab is reloaded, the forward
history will become available again by the forward button.

This means of course that the browser will not be completely 'sanitized' after 

sanitizing the browsing history. 

A real solution would be to create a nsSHistory::PurgeHistory with range
specification like:
purge backward history: PurgeHistory(0, index-1)
purge forward history: PurgeHistory(index, count)

An other solution would be to allow to manually delete all history entries
except for the index, for instance by creating a nsSHistory::removeEntry
function.

Comment 7

[Teune van Steeg]

Attachment: this will fix the reload, and will correctly sanitize the browser history

By adding the history entry at the current index to the end of the history
list, and bringing that entry to view, the session history can be purged
without loosing the current entry.

This should result in a proper sanitization of the session history as well.

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 184161 [details] [diff] [review]
this will fix the reload, and will correctly sanitize the browser history

>Index: toolkit/content/widgets/browser.xml
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/content/widgets/browser.xml,v
>retrieving revision 1.58
>diff -u -r1.58 browser.xml
>--- toolkit/content/widgets/browser.xml	20 May 2005 02:09:09 -0000	1.58
>+++ toolkit/content/widgets/browser.xml	21 May 2005 04:21:38 -0000
>@@ -617,6 +617,15 @@
>             if (aTopic != "browser:purge-session-history")
>               return;
>               
>+            // place the entry at current index at the end of the history list, so it won't get removed
>+            if (this.sessionHistory.index < this.sessionHistory.count-1) {

spaces around - please

>+              var indexEntry = this.sessionHistory.getEntryAtIndex(this.sessionHistory.index, false);
>+              this.sessionHistory.QueryInterface(Components.interfaces.nsISHistoryInternal);
>+              indexEntry.QueryInterface(Components.interfaces.nsISHEntry);
>+              this.sessionHistory.addEntry(indexEntry, true);
>+              var newEntry = this.sessionHistory.getEntryAtIndex(this.sessionHistory.count-1, true);

what's newEntry used for? isn't this line unnecessary?

if we can get a better patch in the next few hours, we can hopefully take this
for a1.  There's probably a better way to handle this in the backend, but
that's just too many questions to answer before we can do it. ;)

Comment 9

[Teune van Steeg]

Attachment: new patch addressing the comments

Indeed, the last line was not necessary. I was thinking it was needed to select
the new entry, but it works fine without selecting it.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 184362 [details] [diff] [review]
new patch addressing the comments

really late for a1, but this fixes a bug in a new feature that we want tested
in this release...

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 184362 [details] [diff] [review]
new patch addressing the comments

a=asa

Comment 12

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050524
Firefox/1.0+ ID:2005052401

WFM

Comment 13

[Asaf Romano (gone)]

Mike checked it in yesterday.

Comment 14

[sairuh (rarely reading bugmail)]

vrfy'd fixed using 2005052410-trunk (deer park) bits on linux fc3.

Comment 15

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

*** Bug 296694 has been marked as a duplicate of this bug. ***

Comment 16

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

*** Bug 247554 has been marked as a duplicate of this bug. ***