972350 - Opening the debugger causes strict warnings on the frontend

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P4)

Description

[Brian Grinstead [:bgrins]]

STR:

* Open browser console and make sure JS warnings are enabled
* Go to http://www.mozilla.org/en-US/
* Open debugger

Notice the warnings that show up:

ReferenceError: reference to undefined property this._editorSource.url     debugger-view.js:378
ReferenceError: reference to undefined property line.textClass             codemirror.js:1020
ReferenceError: reference to undefined property arr[(arr.length - 1)]      codemirror.js:5561
ReferenceError: reference to undefined property line.textClass             codemirror.js:1040
ReferenceError: reference to undefined property aItem.attachment.url       debugger-panes.js:238
ReferenceError: reference to undefined property cc[(cc.length - 1)].lex    javascript.js:230
ReferenceError: reference to undefined property state.globalVars           javascript.js:261
ReferenceError: reference to undefined property cx.state.fatArrowAt        javascript.js:334

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Do we even care about these psuedo strict mode warnings? This is pretty much just style related, since most of this stuff is just checking |if (some.undefinedProperty)|...

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Nick Fitzgerald [:fitzgen] [Ðoge:D6jomNp59N9TVfgc538HU3RswwmwZwcrd7] from comment #1)
> Do we even care about these psuedo strict mode warnings? This is pretty much
> just style related, since most of this stuff is just checking |if
> (some.undefinedProperty)|...

It doesn't cause any issues with execution, but the extra noise in the console makes it harder to find logs that you actually care about.  I don't really care if the code changes or if the console filtering ignores these types of warnings.  But it does seems like these types of warnings could be helpful sometimes.

Comment 3

[Eddy Bruel [:ejpbruel]]

This is just my personal opinion, but I consider it good style to never define properties on an object after they have been constructed. That is, all properties of an object should be defined in its constructor, even if they are initially undefined. This is a habit I picked up once I learned how JS objects are implemented internally (in theory, defining all properties of an object in its constructor allows the type inferencer to make better decisions).

These strict warnings would help catch violations of that rule, but for that to be useful, we would first have to decide that this is a style convention we want to enforce (its something I try to do, but its not official coding style). Even if that's the case, style issues are something we want to catch during review, or fix on the fly, and are otherwise low priority, so I agree with Nick's assessment of P4 here.

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

Not seeing these errors anymore.

Now I'm seeing these, but I don't think I care.

Cc sham for @mozilla.org/io-util;1  debugger.js:2527:8
CC sham for @mozilla.org/scriptableinputstream;1 nsIScriptableInputStream init  debugger.js:2536:8