Reload buttons doesn't work after sanitizing

VERIFIED FIXED in Firefox1.5

Status

()

Firefox
General
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: José Jeria, Assigned: Teune van Steeg)

Tracking

Trunk
Firefox1.5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

13 years ago
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

13 years ago
(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.
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

13 years ago
> 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+

Updated

13 years ago
Flags: blocking1.8b3?
Flags: blocking-aviary1.1?
(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

13 years ago
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


Flags: blocking1.8b2?
(Assignee)

Comment 6

13 years ago
Created attachment 184155 [details] [diff] [review]
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.
(Assignee)

Comment 7

13 years ago
Created attachment 184161 [details] [diff] [review]
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.
Attachment #184155 - Attachment is obsolete: true
Attachment #184161 - Flags: review?(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. ;)
Attachment #184161 - Flags: review?(mconnor) → review-
(Assignee)

Comment 9

13 years ago
Created attachment 184362 [details] [diff] [review]
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.
(Assignee)

Updated

13 years ago
Attachment #184161 - Attachment is obsolete: true
Attachment #184362 - Flags: review?(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...
Attachment #184362 - Flags: review?(mconnor)
Attachment #184362 - Flags: review+
Attachment #184362 - Flags: approval-aviary1.1a1?

Comment 11

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

a=asa
Attachment #184362 - Flags: approval-aviary1.1a1? → approval-aviary1.1a1+
Assignee: nobody → twanno
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → Firefox1.1
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050524
Firefox/1.0+ ID:2005052401

WFM
Mike checked it in yesterday.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
vrfy'd fixed using 2005052410-trunk (deer park) bits on linux fc3.
Status: RESOLVED → VERIFIED

Updated

13 years ago
Flags: blocking1.8b3?
Flags: blocking1.8b2?
Flags: blocking-aviary1.1?
*** Bug 296694 has been marked as a duplicate of this bug. ***
*** Bug 247554 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.