Closed Bug 50656 Opened 24 years ago Closed 24 years ago

window.defaultStatus not working

Categories

(SeaMonkey :: UI Design, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: rcherny, Assigned: law)

References

()

Details

(Keywords: regression, testcase, top100, Whiteboard: [nsbeta3-][rtm++]Fix on trunk, reviewed and approved)

Attachments

(2 files)

From Bugzilla Helper: User-Agent: Mozilla/4.7 [en] (WinNT; U) BuildID: 2000082808 window.defaultStatus is not working. set via onLoad the window status bar should display the requested string text. with an onMouseOver event setting window.status text, it should override the window.defaultStatus display. after the onMouseOver event, when a user mouses out, it should default back to the window.defaultStatus text (onMouseOut is implied). Reproducible: Always Steps to Reproduce: create a page with onload="window.defaultStatus='some text';return true" in the <body> tag. Also create link on page with an onMouseOver window.status set. Actual Results: no text displayed in status bar onLoad. When mouse passed over link, the onMouseOver text does display. OnMouseOut, the window.status remains as default status message. Expected Results: first, onload, it should display the window.defaultStatus text string. onMouseOver does display it's message text, but the message remains onMouseOut (onMouseOut should be implied and the defaultStatus should take over - this was the way it worked in NS 3.x-4.x, and IE4-5.x)
Confirmed with 2000-09-12-12 on Linux.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: 4xp
AFAIK the DOM code does the right thing, reassigning to the docshell people since the DOM code relies on the docshell for setting the default status. Also nominating for beta3 since this is a regression that needs to be fixed.
Assignee: jst → adamlock
OS: Windows NT → All
Priority: P3 → P2
Hardware: PC → All
This is not the docshell at fault here. Calls to set the status, default status, and link are all getting through to the implementation in navigator.js. The implementation of setJSDefaultStatus is here: http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/navigator .js#230 I'm attaching a modified version of the sample that demonstrates status and default status both get through. Reassigning to XP Apps
Assignee: adamlock → don
Component: DOM Level 0 → XP Apps
QA Contact: desale → sairuh
QA Contact: sairuh → claudius
Keywords: testcase
P3, nsbeta3-, assigning to law. Doesn't seem that bad. don suggests cc'ing brendan in case he has feedback.
Assignee: don → law
Priority: P2 → P3
Whiteboard: [nsbeta3-]
Bill, how BAD is this bug anyway? I'm disturbed that it's a regression, but can we actually ship without this? Should it be nominated for rtm?
so something I've been setting in browsers since JavaScript began in Netscape 2.0, "onload=window.defaultStatus" now won't work in Netscape 6? are we going forward or going backwards? I know it isn't a big deal but if our pages are going to be resembling apps more and more it seems rather significant. I know it is trivial in some respects when weighed against others, but small things like this are frequently what products are judged on ... Not that I have my hands in the code under the hood (not a C programmer), but based on some of the attachments here it seems that the code is generally working for defaultStatus, something's just being missed off the onload handler or something. Is that really a difficult fix?
It seems like it wouldn't be too hard to fix. Adam says we're getting the right data to navigator.js. So it appears to just be a problem getting it into the status bar. Might be a symptom of that bug about the load listeners: them firing could trigger us to set (default) status to "Document: Done". But that's just a theory. Might fit with the onload aspect of this report (onload handler is now called just before we show "Document: Done" where previously it was called just afterwards. Again, just a theory. Rob, you're right. This is ugly. But if you had all my bugs, you might not think it was quite so ugly. How about you put some dump() calls in navigator.js and help us figure out what's happening? I can't see giving it to much attention myself (but I'd be glad to help you, or somebody else, work on it).
sorry about my tone it was early/late - wasn't trying to flame. ;) yeah, i know with the volume of bugs you have to pick your battles. i'll check out the nav.js file but i can't make any promises - that will officially be the first mozilla code i've hacked ... and i haven't done any work with dump() yet either ... but i'll look.
Nominating this for rtm so that it gets re-evaluated in case the fix can't make beta 3...
Keywords: rtm
Rob, dump() is just alert() that goes to the console. navigator.js is a good place to start to understand Mozilla; sort of the crossroads where all the technologies come together (i.e., "collide"). Don't hesitate to bug us for help if you need it.
Upping priority to P1, not fixing this for rtm would simply be silly, defaultStatus is used *alot*, adding top100 keyword.
Keywords: top100
Priority: P3 → P1
could this have anything to do with work done on bug 31997 ?
Perhaps related to bug 31997, but more likely just coincidence that the both deal with window.status in one way or another. That one seemed specific to resetting the status upon page load.
nav triage team: RTM NEED INFO so when we get a patch we can + it. P2
Priority: P1 → P2
Whiteboard: [nsbeta3-] → [nsbeta3-][RTM NEED INFO]
Here's the scoop (as I see it, at least): First, the navigator.js code that decides what to display in the status bar (see http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/navigator.js#192) is quite simple. There are 4 sources for this text and this UpdateStatusField code simply uses the first that it finds (i.e., is non-null), in this priority: 1. jsStatus - This is the navigator.js version of the window.status DOM level 0 property, as set by the web page; this has top priority so that setting window.status will change the status and override jsDefaultStatus/defaultStatus (see below) 2. overLink - This is the URL of any moused-over links; it is set by some webshell/docshell notifier mechanism (which essentially calls into navigator.js when the user mouses in/out of links). This is set to null when the user mouses out of a link (which is pertinent for one of the bugs, see below). 3. jsDefaultStatus - navigator.js store for the window.defaultStatus DOM level 0 property; this is updated when scripts set window.defaultStatus 4. defaultStatus - this is a variable that holds what I'd characterize as browser "auxilliary" status messages (such as "Document: Done" and others of that ilk). So the strategy is that the browser shows it's auxilliary msg unless and until the page sets some other msg (via window.status or window.defaultStatus), or, the user mouses over a link. There are specific functions in navigator.js that get called to update each of the first 3 variables. Each calls UpdateStatusField after storing the new value (which makes sense as what is being shown can change whenever any of these 3 variables change state). UpdateStatusField is also called when networking notifications are received (which corresponds with the defaultStatus variable changing value). After adding some debugging code and observing what happens when I go to this bug's attachment, I see two distinct bugs: 1. The setting of window.defaultStatus (i.e., navigator.js's jsDefaultStatus variable) does not "take" because code in the progress notificatioan handling code uses setOverLink(msg) that has the effect of mousing over a link to "Document: Done." The line of code in question is at: http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/navigator.js#329 I think this line of code is in error. If I comment it out, things work just fine: the window.defaultStatus setting in that page's onload handler does what it is supposed to. And I don't see any other problems. That line of code seems to have replaced the commented-out lines that precede it (window.nsXULWindow.setDefaultStatus(msg);). I don't know what that line was supposed to do. It seems guaranteed to break any script that set window.defaultStatus; probably it fixed something funky so that "Document: Done" actually showed up or something like that. 2. The second bug is that when the user mouses *out* of the link with the onMouseOver handler, the status field does not revert to jsDefaultStatus/defaultStatus (as it should, based on empirical evidence of Communicator and IE, and based on text in the JS book I'm using for reference, The Definitive Guide, 3rd Edition). The reason is that when JS code sets window.status, we store that in navigator.js's jsStatus variable and that gets top priority until such time as JS code resets window.status. The problem is that window.status is supposed to be much more transient than that. As The Definitive Guide says: "it is likel to be immediately overwritten by a browser message such as "Document: done". In other words, setting window.status should only cause the status bar to change until such time as some other status-msg-affecting event transpires; e.g., mousing out of the link. To fix *that*, I simply inserted the line: jsStatus = null; inside setJSStatus, just after the call to UpdateStatusField(). With that change, that function now: 1. Updates jsStatus (to null or to what was assigned). 2. Calls UpdateStatusField. This forces the value assigned previously at step 1 to appear on the status bar (presuming it wasn't null). 3. Then sets jsStatus to null. This means jsStatus won't override the other settings the next time UpdateStatusField() is called. When all is said and done, a 2 line of code patch to navigator.js seems to fix things. I'm a little leery of the risk of this patch but I'll throw it out there for people to try out. I'll even try to get it reviewed and approved by the module owner (Ben?). I'll attach the patch shortly (I need to separate that change from other patches I have to navigator.js at the moment).
Attached patch proposed fixSplinter Review
I tried this, and created a test case that used defaultStatus, and the patch works fine. r=ben.
I analyzed the risk factor in that patch and I think it's OK. Here's the rationale that I previously mailed out to some folks here when I asked them to review it: 1. Removing the completely bogus calls to setOverLink is harmless. Since we've put the msg into defaultStatus, we know that text will be used unless some higher priority value is set (jsStatus, overLink, or jsDefaultStatus). Putting it in overLink can only have an effect if jsStatus or jsDefaultStatus is set. jsStatus is never set (with the second patched line, see below). If jsDefaultStatus is set, then we actually want to use it (that's bug #1). 2. Resetting jsStatus to null after the call to UpdateStatusField is OK since that call will have ensured that the text appears in the status bar, at least for a fleeting moment. It will only disappear if something else causes updateStatusField to be called. But in that case, the status field *should* be updated, as per observed behavior of Communicator, IE, and the "spec" (JavaScript, The Definitive Guide) and that is precisely bug #2.
PDT, the fix is now reviewed and approved.
Whiteboard: [nsbeta3-][RTM NEED INFO] → [nsbeta3-][rtm+]Fix in hand, reviewed and approved
r=matt this looks good to me bill
PDT marking [rtm need info] after talking to law on the phone. He's going to land on the trunk and see how it goes before landing on the branch.
Whiteboard: [nsbeta3-][rtm+]Fix in hand, reviewed and approved → [nsbeta3-][rtm need info]Fix in hand, reviewed and approved
Fix checked into the trunk. I looked into my remaining concern, which was that the setOverLink() call to elevate the priority of auxilliary messages such as "Document: Done" might be masking a bug whereby those messages wouldn't appear otherwise. The fear was unfounded, as it turns out. That call does not mask any such bug. Not that there isn't such a bug (see bug 47359). But that line of code certainly doesn't mask it.
Status: NEW → ASSIGNED
PDT folks, this fix is on trunk. As near as we can tell, civilization did not come to an end. :-) Nor did anything else bad happen, at least from this fix. See Bill's previous comment on his former concerns, which are not an issue anymore.
Whiteboard: [nsbeta3-][rtm need info]Fix in hand, reviewed and approved → [nsbeta3-][rtm+]Fix on trunk, reviewed and approved
rtm++.
Whiteboard: [nsbeta3-][rtm+]Fix on trunk, reviewed and approved → [nsbeta3-][rtm++]Fix on trunk, reviewed and approved
Fix checked into branch.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
VERIFIED FIxed branch and trunk builds 2000102008
Status: RESOLVED → VERIFIED
I'm confused. Not to beat an incredibly old and dead horse, but ... I suppose where I'm confused is -- shouldn't the correct behavior reset back to defaultStatus when you mouseOff a link? That's what NS2-4 / IE do ... Here with the current patch (on the original test case) it stays on the mouseOver setting. Am I wrong? moz 1.1a http://www.cherny.com/mozilla/status.html
Claudius, should this be reopened, or a new bug filed? /be
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: