Closed Bug 60628 Opened 24 years ago Closed 24 years ago

javascript strict warnings in tabBindings.xml

Categories

(SeaMonkey :: MailNews: Address Book & Contacts, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: bugzilla, Assigned: maolson)

Details

Attachments

(5 files)

JavaScript strict warning:
chrome://global/content/tabBindings.xml#tabbox.selectedTab (getter) line 7: 
function onget does not always return a value
reassigning to chuang.
Assignee: putterman → chuang
This patch is mostly whitespace and formatting,  the interesing diff is that we
now return 0 as a fallback.
Assignee: chuang → maolson
Keywords: patch, review
cc jag for r=
Status: NEW → ASSIGNED
You can get rid of the getAttribute here. If all is well, that childNode is a
<tab/> which has an xbl property "selected", which does the getAttribute and
setAttribute for you (though in this case it actually does a bit more, see the
actual implementation of it). If not all is well, then trying to access
.selected will cause an exception to be thrown.

Also, elsewhere something similar is done for .selected = false and
removeAttribute, could you fix that while you're at it?
now instead of return 0; we are throwing an exception since there should not eb
a case where we have no selected tab.
We may need to check some of the related code to make sure "no tab
selected" indeed never happens.

Apparantly, tabcontrol's selectedTab setter doesn't check for null, and tabbox's
selectedTab looks slighty broken to me (it will only set the right index if the
tab already was selected). It does however use val.selected meaning setting
selectedTab to null will give you an exception. One problem could be not having
a tab marked as selected in the xul code, so perhaps tabbox's bindingattached
should set the first tab to selected then (and make sure only one tab is
selected)?
Okay, ignore that last bit, I completely missed:

// and also need to select first tab on startup.
...
this.selectedTab = tabs[0];

That's what you get for short nights.

So then I guess we can safely say a tab should be selected at any time and thus
one not being selected is indeed a FAILURE.
Perhaps we should make this more clear by throwing NS_ERROR_NULL_POINTER in the
selectedTab setter(s)?
r=jag
cc'ing alec for sr
Keywords: reviewapproval
can I get a diff -bw? looks like lots of whitespaces fixes
argh, sorry to be a stickler, but I meant cvs diff -bw -u (i.e. still a unified
diff, but ignoring whitespaces - I know that's not obvious but for future
reference, this is what people mean by a "diff -bw" :))
Attached patch -bwu versionSplinter Review
oops, I meant to sr= this yesterday. sr=alecf
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
With Suresh's help, using a debug build, NT4 and build on 3-2-01 this is fixed.
Verified.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: