Closed Bug 157322 Opened 22 years ago Closed 22 years ago

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

Categories

(Core :: DOM: Navigation, defect, P1)

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: 22 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) (deleted) — 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.2 (deleted) — Splinter 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+
Attachment #95778 - Flags: superreview+
Fix checked in, 2002-08-19 11:31/32 PDT.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago22 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.

Attachment

General

Created:
Updated:
Size: