back button not working after applying theme

VERIFIED FIXED in mozilla0.9.1

Status

P3
major
VERIFIED FIXED
18 years ago
14 years ago

People

(Reporter: kgw, Assigned: morse)

Tracking

Trunk
mozilla0.9.1
x86
All

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

18 years ago
From Bugzilla Helper:
User-Agent: Mozilla/4.75 [en] (X11; U; Linux 2.2.16 i686)
BuildID:    2000111421

start up mozilla, then select "view"->"apply theme" and select any of the
available. Now navigate on the displayed website to discover that the back
button will stay disabled.

Reproducible: Always
Steps to Reproduce:
1.apply theme
2.navigate using links from the current page


Actual Results:  back button stays disabled

Expected Results:  back button enabled to go back to where you came from

Comment 1

18 years ago
This WORKSFORME (2000111220 on Win2k)...
What URL were you at when you tried this?  If it was a framed site, this could
be related to bug 56062.
(Reporter)

Comment 2

18 years ago
No. I wasn't using a framed site at that moment (it was the index page for the
qt documentation)

Comment 3

18 years ago
Confirmed build 2000111404 WindowsNT SP5
site: www.ireland.com
opened new browser window, applied classic theme over modern theme, clicked 
onto a random link on the page and the back button does indeed remain greyed out
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Comment 4

18 years ago
I see this bug on build 2000121414 (Linux); I noticed some
other stuff about it.  Here's an example.

Open http://www.mozilla.org in a new window.
Click the link to Mozilla 0.6.
Change themes (or even just apply the one you're using)
via View - Apply theme.
Click the back button.
Click the link to Mozilla 0.6 again.
-- The back button is disabled, the forward button
   looks enabled but doesn't do anything.

OR: after changing themes as above:
Click on the "bugs" link.
Click the back button (at this time it works).
(Notice that both back and forward buttons are
active).
Click on the README link.
-- The forward button is still active, though
it doesn't do anything.
Now click the back button, then the forward button.
-- This time the forward button is inactive.

  It seems that clicking a link will not cause the
status of forward and back buttons to change.  These
properties seem to be shared by the Back and Forward
selections in either the Go menu or the right-click context
menu (they reflect the buttons' status in all cases above).

  Any chance this is related to bug 61991 (URL bar gets
stuck after a theme change)?
nav triage team: looked at this bug, it is not a beta stopper. bulk update of 
several such bugs. 
Keywords: nsbeta1-

Comment 6

18 years ago
My observations of this bug. When you apply a theme and the current window has
the back button disabled, the back button will remain disabled from then on.
This can happen if you have your start page set to a blank page, or if you open
a new window and immediately apply a theme. The back button would normally be
available after going forward a page (or two if you started with a blank
window). This problem occurs regardless of theme.

The Go menu continues to show the history, but for whatever reason the back
button stops responding.

Could this be related to bug 44558? Perhaps something needs to change with the
way themes are applied?

Comment 7

18 years ago
Should have mentioned that I observed this with Build ID: 2001010504 on WinNT
with starting page set to blank. You can reproduce it every time.
I see the problem. 
Target Milestone: --- → mozilla0.9.1

Comment 9

18 years ago
Possibly related/dupe of bug 58416. Thoughts?

Comment 10

18 years ago
*** Bug 58416 has been marked as a duplicate of this bug. ***

Comment 11

18 years ago
Adding to meta skin switching blocker bug and updating to OS ALL based on 
comments here.
Blocks: 68973
OS: Linux → All
(Assignee)

Comment 12

18 years ago
It's unfortunate that bug 58416 got duped to this one and not vice versa since 
that bug had a comment on the cause of the problem.  Therefore repeating that 
comment here:

     Reason this is happening is that the ReinitializeContentVariables() method
     of nsBrowserInstance is not getting called when clicking on links but it
     does get called when you type in a new url.
*** Bug 71095 has been marked as a duplicate of this bug. ***
*** Bug 71279 has been marked as a duplicate of this bug. ***

Comment 15

18 years ago
Related to bug 68662?
I think the comment about ReinitializeContentVariables() getting called for link 
clicks is not valid anymore because nsBrowserInstance.cpp no more holds a 
reference to SessionHistory. I think the problem is in having the right 
nsIwebprogressListener in the Browser object, after a theme change whose 
callbacks update the back and forward buttons.  I think that part of code has 
undergone some recent changes.  Any fix to this problem has to happen in the 
browser area. Giving it to the browser team.  I have a build from 4/9/01 and I 
don't see this bug. However a latest build depicts the problem. 
Assignee: radha → vishy
Status: ASSIGNED → NEW
Component: History: Session → Browser-General
reassigning to morse. 
Assignee: vishy → morse
Keywords: nsbeta1- → nsbeta1
Target Milestone: mozilla0.9.1 → ---
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
Steve, can you investigate and comment on the priority of this bug and whether
it is a nsbeta1+?
(Assignee)

Comment 19

18 years ago
Yes, it should be nsbeta1+ and priority should be at least P2 and maybe P1

Symptoms are much worse than just the back button.  Nothing in the navigator 
window (forward button, url bar, etc) are getting updated after a theme switch.

Radha was correct in his assessement.  When the theme switch occurs, all the 
listeners are removed from the old mListenerList (see nsDocLoader.cpp), a new 
mListenerList is created, and all the listeners are reinserted into that new 
list.  All listeners that is, except for one.  That one being the listeners in 
navigator.js.
(Assignee)

Comment 20

18 years ago
Here's a status update on this bug report.

The following lines of code will fix the problem:

var interfaceRequestor = getBrowser().docShell.QueryInterface
   (Components.interfaces.nsIInterfaceRequestor);
var webProgress = interfaceRequestor.getInterface
   (Components.interfaces.nsIWebProgress);
webProgress.addProgressListener(window.XULBrowserWindow);

I can demonstrate this by putting these lines of code into the BrowserReload 
function of navigator.js and then clicking on the reload button after the theme 
switch.  Once I do that, alll updates are occuring correctly.

So now I need to figure out a way of having that code automatically executed 
after the theme switch.  Note that it will not work to simply drop that code 
into the applyTheme function of navigator.js because that is too early -- the 
code must be executed *after* the next initialization occurs in nsDocLoader.
(Assignee)

Comment 21

18 years ago
OK, I have something that works.  Basically I set up onclick and onkeypress 
handlers when the theme-switch occurs so that I can get control after the theme 
switch.  And at that time I reinstall the progress listeners for the new doc 
loader.

Note that I can wait for the next keypress or mouseclick because nothing in the 
browser window will change state until that happens.  For example, the back 
button won't need to be updated until the user goes to another url and he needs 
to either type in the URL field for that (keypress), tab to a link and press 
enter (keypress), or click on a link (mouseclick).

Attaching patch that accomplishes this.
(Assignee)

Comment 22

18 years ago
Created attachment 32193 [details] [diff] [review]
patch to update progress listeners after theme switch
(Assignee)

Comment 23

18 years ago
r=matt
(Assignee)

Comment 24

18 years ago
cc'ing alecf for super-review
(Assignee)

Comment 25

18 years ago
Actually let me make a slight modification to the patch posted.  The patch 
attaches the onclick and onkeypress handles to the content area.  But that 
doesn't include the urlbar area, so we won't get control when the user navigates 
by typing in a new url.

Therefore the correction to the patch is to replace:

   var content = document.getElementById("appcontent");

with

   var toolbox = document.getElementById("navigator-toolbox");

and of course the next lines that start with "content." would start with 
"toolbox." instead.

Comment 26

18 years ago
I don't understand why you have to use onclick and onkeypress listeners. 
Couldn't you just set a timeout following the applytheme function and then just
rehook up the listener in the timeout callback?
(Assignee)

Comment 27

18 years ago
BTW, this patch fixes all the following bugs as well:

  bug 61796: Progress meter not working after theme switch (ben)
  bug 61853: Can't enter text in url field after theme switch (hyatt)
  bug 61991: url bar not updated after theme switch (alecf)
  bug 68227: status bar not updated after theme switch (ben)
  bug 68230: throbber stops working after theme switch (pchen)
(Assignee)

Comment 28

18 years ago
Isn't that kind of lame to use a timeout?  By attaching the handlers I get 
called at just the time that I need to make the adjustment.  What if the timeout 
occurs too quickly?  What if it takes too long and the user already clicked on a 
link?
setAttribute("onclick", "foopy");
&
setAttribute("onclick", "");

is probably better as

addEventListener("click", foopy, false);
&
removeEventListener("click", foopy, false);

I think jag was meaning to look into something related to this to see if he
broke something elsewhere, or failed to fix it. If he comes up blank, sr=ben. 
(Assignee)

Comment 30

18 years ago
Attaching my latest patch for reference.  It's my original patch together with 
the correction that I indicated.  It also has ben's suggested change in it but 
commented out for the time being.

Reason I am posting this is because suddenly this is no longer working for me.  
I'm sure it worked earlier when I originally tested it.  By having the patch 
here I can ask someone else (namely jag) to apply it and see if it works for 
him.
(Assignee)

Comment 31

18 years ago
Created attachment 32283 [details] [diff] [review]
corrected patch

Comment 32

18 years ago
Created attachment 32284 [details] [diff] [review]
[patch] Fix. Use js web progress listener, not old (dead) appCore one.

Comment 33

18 years ago
This should fix most issues, though there still is an issue on the uri bar not
updating. It looks like I'm somehow sending the new values to an old textfield,
but that seems to be a different issue.
(Assignee)

Comment 34

18 years ago
Patch works.  r=morse
(Assignee)

Comment 35

18 years ago
jag: with your patch the uri bar is updating fine for me.
(Assignee)

Comment 37

18 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 38

18 years ago
*** Bug 68227 has been marked as a duplicate of this bug. ***

Comment 39

18 years ago
VERIFIED Fixed with 2001053104 builds
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.