Closed Bug 103264 Opened 22 years ago Closed 22 years ago

Site Navigation Bar doesn't activate on startup page.

Categories

(SeaMonkey :: UI Design, defect)

Product:
SeaMonkey
Component:
UI Design
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: choess, Assigned: choess)

References

(
URL
)

Details

Attachments

(2 files)

(kludgy?) patch to solve the problem.
1.60 KB, patch
Details | Diff | Splinter Review
Better, non-kludgy patch
1.94 KB, patch
nobody
: review+
nobody
: superreview+
asa
: approval+
Details | Diff | Splinter Review 
The toolbar doesn't show anything for the page the browser starts up in,
regardless of links on the page.  Probably a side-effect of frantic perf
patching.  Stuart, I'm cc'ing you and Hyatt on this since you've become our
performance gurus; I'll let the rest of the regulars trickle in as they please.
Blocks: 103053
QA Contact: sairuh → claudius
Hmm. To get the toolbar to work again, I have to switch it off (I have it
auto-on in preferences) and then back on.  Seems to be associated with a js
error: "document.getElementbyID("linktoolbar") has no properties", line 66 of
linkToolbarOverlay.js.
"document.getElementbyID("linktoolbar") has no properties" doesn't make sense to
me unless we're somehow being called in a context where "document" refers to the
content document rather than the chrome. Is there any way to get a stack trace
out of JS?

If I get a chance I'll look into this today.
Evidently I should clarify.  The toolbar appears when I open a new window 
(which it shouldn't, for some sites; bug 103273), but no links are active in 
it.  As I continue browsing, the toolbar remains, but inactive, until I switch 
it off, then on again.
Does that include browsing to a site with links on? Does the toolbar remain
inactive even if it should be active?
Same behavior occurs with "Open in New Window."  If the Site Navigation Bar is
set to "Show Always" or "Show Only As Needed," any new window will have the
toolbar visible but disabled.  This persists with browsing to other pages,
whether they would normally activate the toolbar or not.  The only way this
changes is when you go to the "View" menu (in that same window) and select the
mode for the toolbar - and it works even if it's the same mode you already have.
 Once that is done, the toolbar either disappears or activates, and from then on
acts normally within that window.

Got it (I think) - this is happening because the document.getElementById is
called (indirectly) from script that appears higher up the overlay than the
declaration of the element with that ID. I'm asking around on
#mozilla/#mozillazine for a way to do this right (presumably the equivalent of
<body onload> in html) but although <window> is supposed to have an onload
property, I'm not sure how I'd go about setting it from an overlay.

Would:

document.addEventListener("load", function() { ... })

work? Or would that also capture all the bubbled up load events from the content?

The attached patch solves the problem but works by adding code to
navigator.js/Startup() which breaks the self-containedness of the toolbar.

I'm also not 100% sure that Startup() only gets called once - things like the
(!cycling) check in it suggest otherwise.

Hyatt, as XUL god, what are your thoughts on the "right" way to make a one-time
initialization happen after the XUL document and overlay have fully loaded?
By the way, if Startup() *is* the right thing to do here, the exact location in
Startup() should be chosen a heck of a lot more carefully than I chose it. So
this patch is certainly not ready for primetime.
Attachment #52234 - Flags: needs-work+
You should put inline script in the overlay that adds a load listener to the
chrome.  Then that function will be called after the window is fully loaded, and
you can do your initialization then.

The following patch adds the handler in the way suggested by Hyatt, and works :)

I also added code to ensure that this temporary onload handler gets removed
after the first time it's called, just in case we get bubbled load events from
the page itself - we definitely don't want to do this on every pageload :)

IMO, this patch is ready for consideration for r/sr, and we should look to get
a= for 0.9.5 because the toolbar doesn't work right in any mode except disabled
without this.

This bug was my fault, I suck, I should test better before submitting patches...
sorry to everyone.
sr=hyatt

*** Bug 103273 has been marked as a duplicate of this bug. ***
Comment on attachment 52254 [details] [diff] [review]
Better, non-kludgy patch

r=gerv.

Gerv
Attachment #52254 - Flags: superreview+
Attachment #52254 - Flags: review+
Comment on attachment 52254 [details] [diff] [review]
Better, non-kludgy patch

a=asa for checkin to 0.9.5 branch
Attachment #52254 - Flags: approval+
Checked in by Gerv to both branch and trunk.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
*** Bug 103790 has been marked as a duplicate of this bug. ***
*** Bug 103721 has been marked as a duplicate of this bug. ***
mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31.

if you think this particular bug is not fixed, please make sure of the following
before reopening:

a. retest with a *recent* trunk build.
b. query bugzilla to see if there's an existing, open bug (new, reopened,
assigned) that covers your issue.
c. if this does need to be reopened, make sure there are specific steps to
reproduce (unless already provided and up-to-date).

thanks!

[set your search string in mail to "AmbassadorKoshNaranek" to filter out these
messages.]
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.