If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

javascript strict warnings in EdPageProps.js; can't OK dlog

VERIFIED FIXED in mozilla0.9.4

Status

()

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

People

(Reporter: Henrik Gemal, Assigned: Charles Manske)

Tracking

Trunk
mozilla0.9.4
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
Warning: assignment to undeclared variable dialog
Source File: chrome://editor/content/EdPageProps.js
Line: 41


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

Comment 1

17 years ago
cmanske
Assignee: beppe → cmanske
Priority: -- → P3
Target Milestone: --- → mozilla0.9.3

Comment 2

17 years ago
one line fix, removal of js error

reviewed and approved
Keywords: nsBranch

Comment 3

16 years ago
removing nsBranch and pushing out to 1.0
Keywords: nsBranch
Target Milestone: mozilla0.9.3 → mozilla1.0
Created attachment 43053 [details] [diff] [review]
Declares the variable (before, it was just initialized without declaration)

Comment 5

16 years ago
Thanks! r=akkana

Comment 6

16 years ago
sr=blake

That if (!dialog) check is, uh, interesting...
C:\moz_src\mozilla\editor\ui\dialogs\content>cvs commit -m "86706.  Javascript s
trict warning in EdPageProps.js.  r=akkana@netscape.com sr=blaker@netscape.com"
EdPageProps.js
Checking in EdPageProps.js;
/cvsroot/mozilla/editor/ui/dialogs/content/EdPageProps.js,v  <--  EdPageProps.js

new revision: 1.17; previous revision: 1.16
done
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 8

16 years ago
reopening this bug; I don't think that this fix was correct.
I can't dismiss the dialog with the OK button.
Did anyone test this before it was checked in?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 9

16 years ago
What is the error you're getting?

I don't see how this fix could be incorrect unless dialog is declared elsewhere
within that scope.  That's impossible to tell from the context, so I assumed
stephen checked for that first.  I did say that that !dialog check was
interesting.  That may be messing things up (what the heck is the point of it?),
but I don't see why stephend's change would have exposed that problem.

Comment 10

16 years ago
Charley knows this code best; dialog is decleared elsewhere.  StephenD's fix 
changed the scope so other functions in that file aren't setup/initialized 
properly.

The current bug is that you can't "OK" the dialog.
Summary: javascript strict warnings in EdPageProps.js → javascript strict warnings in EdPageProps.js; can't OK dlog
I'm so sorry.  Looks like that variable is declared as a global somewhere else,
and declaring it within this function creates a local, which obviously is the
reason for it breaking.  Backing out my "fix" for the JS warnings returns the OK
button's callback to work again.  Brade, can you r= a new fix to go back to the
old code?
Created attachment 43434 [details] [diff] [review]
Fix.  I'm prepared for my well-deserved public flogging.
(Assignee)

Comment 13

16 years ago
Created attachment 43446 [details] [diff] [review]
The correct fix to avoid the JS warning.
(Assignee)

Comment 14

16 years ago
Sorry I didn't see this bug before fix went in! Just catching up on recent work.
"dialog" is a global in all the Composer property dialogs. I plan to replace
all those with "gDialog" to reduce confusion.

Status: REOPENED → ASSIGNED
It's me that needs to apologize, again this won't happen again.  Sorry.

Comment 16

16 years ago
Can we merge both of these patches into one patch?  After that and some testing, 
r=brade.

Charley--please call it "gComposerDialog" or something specific to editor dialogs 
(in case some other app wants to use gDialog).  I'll file a new bug for this.

Thanks to Sujay for discovering this problem so quickly!
Whiteboard: fix in hand; need sr=
(Assignee)

Comment 17

16 years ago
Created attachment 43580 [details] [diff] [review]
Combined patches as brade suggested

Comment 18

16 years ago
sr=kin@netscape.com

Updated

16 years ago
Whiteboard: fix in hand; need sr= → fix in hand; waiting tree opening
(Assignee)

Comment 19

16 years ago
changing milestone to 0.9.4
Target Milestone: mozilla1.0 → mozilla0.9.4

Comment 20

16 years ago
*** Bug 92714 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 21

16 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
Whiteboard: fix in hand; waiting tree opening
*** Bug 94844 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 23

16 years ago
Verified! Warnings seems gone to me. Will reopen if found again...
Build 20010820 on win2k
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.