javascript strict warnings in editor.js

VERIFIED FIXED in mozilla0.9

Status

()

Core
Editor
P3
normal
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: Henrik Gemal, Assigned: Charles Manske)

Tracking

Trunk
mozilla0.9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

17 years ago
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

17 years ago
move to Editor component
Assignee: kmcclusk → beppe
Component: Compositor → Editor
QA Contact: petersen → sujay

Comment 2

17 years ago
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.
Assignee: beppe → sfraser
Target Milestone: --- → mozilla0.9
(Reporter)

Updated

17 years ago
Summary: Lots of strict warnings in editor.js → javascript strict warnings in editor.js

Comment 3

17 years ago
reassigning to charley
Assignee: sfraser → cmanske
(Assignee)

Comment 4

17 years ago
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.

Target Milestone: mozilla0.9 → Future

Comment 5

17 years ago
cmanske: you can avoid the warning by writing

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

Comment 6

17 years ago
Created attachment 22705 [details] [diff] [review]
Fix uses Simon's excellent suggestion

Comment 7

17 years ago
r=blake

Comment 8

17 years ago
@@ -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

17 years ago
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)
(Assignee)

Comment 10

17 years ago
Thanks, my further testing revealed that I also needed to test for value.
New attachment comming -- doing testing concerning related problem.
Status: NEW → ASSIGNED
Target Milestone: Future → mozilla0.8
(Assignee)

Comment 11

17 years ago
Created attachment 22743 [details] [diff] [review]
A better fix - tests for non-null InsertCharWindow

Comment 12

17 years ago
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...
(Assignee)

Comment 13

17 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 14

17 years ago
verified in 1/24 buld.
Status: RESOLVED → VERIFIED
(Reporter)

Comment 15

17 years ago
in build 20010328

JavaScript strict warning:
chrome://editor/content/editor.js line 538: reference to undefined property 
menuItem.value
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 16

17 years ago
Created attachment 29084 [details] [diff] [review]
Fix
(Assignee)

Comment 17

17 years ago
ready for review
Status: REOPENED → ASSIGNED
Keywords: review
Whiteboard: FIX IN HAND need r=, sr=
Target Milestone: mozilla0.8 → mozilla0.9

Comment 18

17 years ago
sr=sfraser

Comment 19

17 years ago
r=brade
Whiteboard: FIX IN HAND need r=, sr= → FIX IN HAND
(Assignee)

Comment 20

17 years ago
Thanks.
Removing requests for reviews.
Keywords: review
(Assignee)

Comment 21

17 years ago
checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
Whiteboard: FIX IN HAND

Comment 22

17 years ago
Henrik, is this fixed for you in recent builds ? please mark verified-fixed.
thanks.
(Reporter)

Comment 23

17 years ago
warning is now gone...
Status: RESOLVED → VERIFIED
(Reporter)

Comment 24

17 years ago
JavaScript strict warning:
chrome://editor/content/editor.js line 2021: reference to undefined property
Components.classes['@mozilla.org/spellchecker;1']
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 25

17 years ago
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

17 years ago
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

17 years ago
Rather than morphing this bug into something about the spellchecker, could we
please file a bug if this is reproduceable?
(Assignee)

Comment 28

17 years ago
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.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 29

17 years ago
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);
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---

Comment 30

17 years ago
Henrik--please file a new bug on that particular issue.  Thanks!
re-resolve as fixed
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 31

17 years ago
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...
(Reporter)

Comment 32

17 years ago
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);
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.