57649 - javascript strict warnings in editor.js

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Henrik Gemal]

JavaScript strict warning:
chrome://editor/content/editor.js line 266: reference to undefined property colo
rs.TextColor

JavaScript strict warning:
chrome://editor/content/editor.js line 269: reference to undefined property colo
rs.BackgroundColor

JavaScript strict warning:
chrome://editor/content/editor.js line 2174: reference to undefined property win
dow.InsertCharWindow

JavaScript strict warning:
chrome://editor/content/editor.js line 2216: reference to undefined property tem
pWindow.InsertCharWindow

JavaScript strict warning:
chrome://editor/content/editor.js line 2216: reference to undefined property tem
pWindow.InsertCharWindow

JavaScript strict warning:
chrome://editor/content/editor.js line 2216: reference to undefined property tem
pWindow.InsertCharWindow

JavaScript strict warning:
chrome://editor/content/editor.js line 2174: reference to undefined property win
dow.InsertCharWindow

JavaScript strict warning:
chrome://editor/content/editor.js line 2216: reference to undefined property tem
pWindow.InsertCharWindow

JavaScript strict warning:
chrome://editor/content/editor.js line 2216: reference to undefined property tem
pWindow.InsertCharWindow

JavaScript strict warning:
chrome://editor/content/editor.js line 2216: reference to undefined property tem
pWindow.InsertCharWindow

Comment 1

[Kathleen :Brade]

move to Editor component

Comment 2

[rubydoo123]

assigning to sfraser, simon assigning to you as part of the embedding work,
which will probably include a lot of this type of clean-up.

Comment 3

[rubydoo123]

reassigning to charley

Comment 4

[Charles Manske]

First, please tell us what action was taken to generate these warnings.
The colors.TextColor and .Background warnings were fixed.
I cannot fix the reference to "InsertCharWindow". If the Insert Character dialog
exists, this window attribute holds that window. So when we don't have that
dialog, the window attribute is "undefined". I see nothing wrong with testing
if "window.InsertCharWindow" exists -- that's what generates the warning.

Comment 5

[Simon Fraser [no longer active]]

cmanske: you can avoid the warning by writing

   if ("InsertCharWindow" in window)
     window.InsertCharWindow .... etc.

Comment 6

[Charles Manske]

Attachment: Fix uses Simon's excellent suggestion

Comment 7

[Blake Ross]

r=blake

Comment 8

[timeless]

@@ -2246,7 +2246,7 @@
<timeless> jag: if i have a field that satisfies ("a" in b), can b.a be null?
<jag> yes
please check for null. or comment explaining why it could never happen.

Comment 9

[Peter "jag" Annema]

Note that changing
  if (window.InsertCharWindow)
to
  if ("InsertCharWindow" in window)

changes its functionality. In the first case you check whether the property
isn't null, in the second case you only check whether the window object has a
property called "InsertCharWindow". If an existance check is all you need, then
this patch is fine. If you need to know it exists and isn't null, change those
lines to something like:

  if ("InsertCharWindow" in window && window.InsertCharWindow)

Comment 10

[Charles Manske]

Thanks, my further testing revealed that I also needed to test for value.
New attachment comming -- doing testing concerning related problem.

Comment 11

[Charles Manske]

Attachment: A better fix - tests for non-null InsertCharWindow

Comment 12

[timeless]

r=timeless

+  if (windowWithDialog && "InsertCharWindow" in windowWithDialog && 
+      windowWithDialog.InsertCharWindow)
...
+               if (tempWindow != window && "InsertCharWindow" in tempWindow && 
tempWindow.InsertCharWindow)

if you're concerned about long lines, you might want to change the longer 
lines...

Comment 13

[Charles Manske]

checked in

Comment 14

[sujay]

verified in 1/24 buld.

Comment 15

[Henrik Gemal]

in build 20010328

JavaScript strict warning:
chrome://editor/content/editor.js line 538: reference to undefined property 
menuItem.value

Comment 16

[Charles Manske]

Attachment: Fix

Comment 17

[Charles Manske]

ready for review

Comment 18

[Simon Fraser [no longer active]]

sr=sfraser

Comment 19

[Kathleen :Brade]

r=brade

Comment 20

[Charles Manske]

Thanks.
Removing requests for reviews.

Comment 21

[Charles Manske]

checked in.

Comment 22

[sujay]

Henrik, is this fixed for you in recent builds ? please mark verified-fixed.
thanks.

Comment 23

[Henrik Gemal]

warning is now gone...

Comment 24

[Henrik Gemal]

JavaScript strict warning:
chrome://editor/content/editor.js line 2021: reference to undefined property
Components.classes['@mozilla.org/spellchecker;1']

Comment 25

[Charles Manske]

This line is how we detect if the spellcheck is installed:
  var spellcheckerClass = Components.classes["@mozilla.org/spellchecker;1"];
I don't think there's any way around this warning when spellchecker isn't
installed.
Kin? Is this correct? Please mark "invalid" if yes.

Comment 26

[kinmoz]

No, we shouldn't see any JS errors. I don't see that in my Mozilla Win32 debug
build from this morning either, and I don't have the spellchecker installed.

Comment 27

[kinmoz]

Rather than morphing this bug into something about the spellchecker, could we
please file a bug if this is reproduceable?

Comment 28

[Charles Manske]

Kin doesn't see this in today's debug build, but if you do see a warning and
are concerned about it, please file a new bug. It's hard to wade throught the
old problems to find the newest one. Thanks.

Comment 29

[Henrik Gemal]

this is still in todays (20010509) build.
Please note:
this is not a javascript error but a warning, which is *only* seen with:
user_pref("javascript.options.strict", true);

Comment 30

[Kathleen :Brade]

Henrik--please file a new bug on that particular issue.  Thanks!
re-resolve as fixed

Comment 31

[Henrik Gemal]

I've been told by Netscape engineers that when warnings appear in already 
reported (but perhaps fixed) files I should reopen bug instead of open a new. 

But I can also open a new one...

Comment 32

[Henrik Gemal]

I cant see the warnings with todays build 20010619.
Will reopen if I see them again.

Please add the following line to your prefs.js file, so we could avoid all 
these strict warning fixup...:
user_pref("javascript.options.strict", true);