Closed Bug 157322 Opened 18 years ago Closed 17 years ago

Back Button loses history when toolbars are collapsed (classic skin)

Categories

(Core :: Document Navigation, defect, P1, major)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla1.2alpha

People

(Reporter: chrisowens, Assigned: dbaron)

References

Details

(Whiteboard: [patch])

Attachments

(1 file, 1 obsolete file)

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1a+) Gecko/20020713
BuildID:    20020713

If I collapse the "Navigation Toolbar" and the "Personal Toolbar", and then
"un-collapse" them, the back button has lost it's history and no longer
functions for the rest of the browser session.

Reproducible: Always
Steps to Reproduce:
1. If I collapse the "Navigation Toolbar" and the "Personal Toolbar"
2. Then "un-collapse" them
3. The back button has lost it's history and no longer functions for the rest of
the browser session.

Actual Results:  The back button (maybe forward too) stop functioning.

Expected Results:  Mozilla should have remembered the history of both the back
and forward buttons.

No other information is needed, as all steps have been detailed in the other
form elements.
same problem with win98 build 2002071308
Confirming on Linux/i686 (trunk build 2002071622).

It is sufficient to collapse and uncollapse the Navigation Toolbar to see
this. Only the current window is affected, new windows will work normally.
This error is printed on the JS console when back/forward button is clicked:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIWebNavigation.sessionHistory]"  nsresult:
"0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://navigator/content/sessionHistoryUI.js :: FillHistoryMenu :: line 54" 
data: no]
Status: UNCONFIRMED → NEW
Ever confirmed: true
-> History:Session
Assignee: Matti → radha
Component: Browser-General → History: Session
QA Contact: asa → claudius
*** Bug 157700 has been marked as a duplicate of this bug. ***

*** This bug has been marked as a duplicate of 150749 ***
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → DUPLICATE
not a dupe of bug 150749. sorry. 
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
*** Bug 158116 has been marked as a duplicate of this bug. ***
*** Bug 158321 has been marked as a duplicate of this bug. ***
I'm pretty sure that this bug is the result of changes that went in for bug
156719 (which was a fix for a regression from the fix at bug 143862). I think
the plan is to back out the changes from bug 143862 and bug 156719 (and reopen
them) which fixes this problem. This bug can then be resolved.
Assignee: radha → dbaron
Status: REOPENED → NEW
I don't see how the changes for bug 156719 would have caused this.  They only
changed a codepath that would have caused assertions before.  Did someone test
that the backout was actually relevant?
I've put the changes that were backed out back in my tree, and I can't reproduce
this bug, bug 157700, bug 158116, or bug 158321.  Could someone clarify the
steps to reproduce?  Or was the backout unrelated to this bug and the bugs
marked duplicates of it?  I don't see anyone saying that they tested the backout
to see if it fixed this bug.
Oh, er, I can reproduce it in the Classic theme but not in Modern.  Never mind.
Summary: Back Button loses history when toolbars are collapsed → Back Button loses history when toolbars are collapsed (classic skin)
Attached patch patch (obsolete) — Splinter Review
So what was happening was that we were doing a frame reconstruct for the frame
tree for the main XUL document in the navigator window since the ESM was
sending a ContentStateChanged notification on content that wasn't in the
document.

This patch fixes the ESM not to send such notifications.  I'll attach a revised
patch to bug 143862 as well, to make that document check a check in optimized
builds rather than just an assertion.

(It seems that this was a regression from bug 143862, not bug 156719.)
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [patch]
Target Milestone: --- → mozilla1.2alpha
In a review request for this bug, I wrote:

  This was a regression caused by my checkin for bug 143862, which was
  backed out because of the regression, but which I'd like to reland.
  I've fixed the bug in two ways:
   1) putting a null-check in the patch for bug 143862 that I intend to
      reland
   2) This patch.

  The patch for bug 143862 fixed things so that we would handle a
  frame-reconstruct on the root element.  The problem with that patch
  was that other things can look like the root element (by having no
  parent) if they've been removed from the document.  The null check in
  the patch to bug 143862 ensures that they really are the root element.
  This patch prevents the event state mananger from sending
  ContentStateChanged notifications for content that has been removed
  from the document by using its ContentRemoved notification (which is
  called from PresShell::ContentRemoved) to clear out its mHoverContent
  (by switching to the parent) and its mActiveContent and
  mDragOverContent (by nulling).

  Also, I think esm->ContentRemoved() should also be called from
  PresShell::ContentReplaced.  Do you think it's dangerous to make that
  change considering what nsEventStateManager::ContentRemoved does?
Comment on attachment 95772 [details] [diff] [review]
patch

r=bryner
Attachment #95772 - Flags: review+
Attached patch patch, v.2Splinter Review
This fixes the ContentReplaced issue from my previous comment.
Attachment #95772 - Attachment is obsolete: true
Comment on attachment 95778 [details] [diff] [review]
patch, v.2

r=bryner
Attachment #95778 - Flags: review+
Comment on attachment 95778 [details] [diff] [review]
patch, v.2

sr=kin@netscape.com
Attachment #95778 - Flags: superreview+
Fix checked in, 2002-08-19 11:31/32 PDT.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago17 years ago
Resolution: --- → FIXED
Component: History: Session → Document Navigation
QA Contact: claudius → docshell
You need to log in before you can comment on or make changes to this bug.