66928 - javascript strict warnings in toolbarBindings.xml

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Henrik Gemal]

The console is flooded with warninsg like this:

JavaScript strict warning:
chrome://global/content/toolbarBindings.xml#menubar.findNodeByAttribute() line 1
1: function findNodeByAttribute does not always return a value

JavaScript strict warning:
chrome://global/content/toolbarBindings.xml#toolbargrippy.collapsed (getter) lin
e 2: function onget does not always return a value

JavaScript strict warning:
chrome://global/content/toolbarBindings.xml#toolbargrippy.collapsed (setter) lin
e 3: function onset does not always return a value

JavaScript strict warning:
chrome://global/content/toolbarBindings.xml#toolbargrippy.returnNode() line 7: f
unction returnNode does not always return a value

JavaScript strict warning:
chrome://global/content/toolbarBindings.xml#toolbar.findNodeByAttribute() line 1
1: function findNodeByAttribute does not always return a value

Comment 1

[Mark Olson]

I am seeing this too, but wasn't sure it wasn't my build.  This appears to have
happened over the weekend.

Comment 2

[Mark Olson]

cc'ing brendan, since he had some js changes over the weekend.  Brendan, if
you're not at fault, maybe you could offer some insight into why this and
several other similar strict warnings popped up very recently (i.e. who gets the
real blame?).

This is making fixing other strict warnings a real challenge, since there is
considerable console spew.

Comment 3

[Mark Olson]

Ok, backing out brendan's patch for bug 33390 in my local tree stops these
warnings, as well as the myriad others that appeared around the same time.

Brendan, with strict warnings turned on, this is apparent opening most (all?)
xul windows.  It appears to affect .xml files primarily (only?), as I haven't
seen an unexplainable jump in warnings in xul .js files.  This make the console
almost useless for debugging with strict warnings on.

Comment 4

[Brendan Eich [:brendan]]

Argh.  bug 33390 was a compiler footprint reduction fix, not obviously to do
with strict warnings.  I'll fix whatever went wrong; sorry about that.

Comment 5

[Brendan Eich [:brendan]]

*** Bug 66930 has been marked as a duplicate of this bug. ***

Comment 6

[Brendan Eich [:brendan]]

I can't reproduce this.  It doesn't make sense to me, either.  Can someone with
a debug build and a debugger reproduce it and help me remote-debug it?

/be

Comment 7

[Henrik Gemal]

just opening a new window with 2001020104 floods my console with strict warnings 
including those about toolbarBindings.xml.
This is with strict option on.

Or wasn't this what you asked about..?

Comment 8

[Brendan Eich [:brendan]]

Got it (I was being lame earlier).  XBL-compiled functions partake of the space
savings from the fix for bug 33390, which means each statement in the top-level
block statement that is the function's body is parsed and then compiled into
bytecode, then recycled.  This is a win, but it confuses the CheckFinalReturn
code.  Patch coming up, cc'ing r= and sr= buddies.

/be

Comment 9

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 10

[Brendan Eich [:brendan]]

Attachment: better patch, common code subroutining, Has rather than Check for report-free predicate naming convention

Comment 11

[Brendan Eich [:brendan]]

Attachment: old-style diff of first two patches, so reviewers can see the minimal changes

Comment 12

[Mike Shaver (:shaver emeritus)]

Looks like a good fix, and cleanup to boot!

sr=shaver

Comment 13

[John Bandhauer]

r=jband

Comment 14

[Brendan Eich [:brendan]]

Fix checked in.  Thanks,

/be

Comment 15

[Henrik Gemal]

Since bug 66930 was named a dupe of this bug.

JavaScript strict warning:
XStringBundle.stringBundle (getter) line 1: reference to undefined property this
._bundle

JavaScript strict warning:
XStringBundle.stringBundle (getter) line 1: reference to undefined property this
._bundle

Comment 16

[Brendan Eich [:brendan]]

Henrik: please don't reopen this bug based on any old strict warning that crops
up.  Those warnings are legit; jag is on the case.

/be

Comment 17

[sairuh (rarely reading bugmail)]

no longer see this when i open a new browser window [ctrl+n]. vrfy using
2001.02.09.08 comm bits on linux.

i do see other errors, but that's covered by bug 61708.