195241 - In <chrome://editor/content/editorUtilities.js>, "Error: redeclaration of const XUL_NS"

Status: RESOLVED WORKSFORME

(SeaMonkey :: Composer, defect)

Description

[Serge Gautherie (:sgautherie)]

User-Agent:       Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.3a) Gecko/20021212
Build Identifier: Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.3a) Gecko/20021212

(I did not identify any related misbehaviour.)


Reproducible: Didn't try

Steps to Reproduce:
(I don't know how to reproduce it.)

Actual Results:  
[
Error: redeclaration of const XUL_NS
Source File: chrome://editor/content/editorUtilities.js
Line: 1
]



In the JS console, this message was
*after "redeclaration of const MOZ_HELP_URI" (see bug 162432)
*before "this.docShell has no properties" (see bug 104360)

I don't know if the 3 are related, because I opened the JS console afterward.

NB: I was using Mozilla and MailNews at the time.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Editor.  Upping severity, since console spew makes real problems hard to diagnose.

Comment 2

[Serge Gautherie (:sgautherie)]

Attachment: (Part B) <toolbar.xml> cleanup [Moved to bug 225328]

(found while looking at this bug)

Part B: "<toolbar.xml> cleanup"

*Removes unused 'XUL_NS': that should be all right :-)

*Adds a 'throw': otherwise, we should comment on why we want the 'catch' to
remain empty :-|
*Additional question about 'catch' or no 'catch'
 in
<http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/bindings/toolbar.xml>

 *line 92: "<method name="createCollapsedGrippy">" + catch { throw }
 *line 45: "this.createCollapsedGrippy(toolbar);" + catch {}
 *line 158 & 263: "this.parentNode.createCollapsedGrippy(this);" + no catch
Should some catch/throw be added/removed, or is it as it has to be ?

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 135198 [details] [diff] [review]
(Part B) <toolbar.xml> cleanup
[Moved to bug 225328]


Neil, Can you have a look at the related question in comment 2 ?

Comment 4

[Serge Gautherie (:sgautherie)]

Attachment: (Part C) <no_space.js> cleanup [File removed]

(found while looking at this bug)

Part C: "<no_space.js> cleanup"

*Removes 'XUL_NS':
 *no_space.js
   is included in no_space.xul
    which is referenced in nsPrefMigration.cpp
 *The 'var' is unused in theses 3 files: good to remove ?
  If not, it should be changed to 'const' ! or retrieved from elsewhere ??

*The rest is only reformatting.

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 135203 [details] [diff] [review]
(Part C) <no_space.js> cleanup [File removed]


Neil, Can you have a look at the related question in comment 4 ?

Comment 6

[Serge Gautherie (:sgautherie)]

Part A: "redeclaration"

LXR search result, besides Part-B and Part-C patchs:
{
'XUL_NS' is included in

/editor/ui/composer/content/editorUtilities.js, line 27 -- const XUL_NS =
"http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
/xpfe/global/resources/content/bindings/toolbar.xml, line 96 -- const XUL_NS =
"http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";

/xpfe/components/bookmarks/resources/bookmarks.js, line 38 -- var NC_NS, WEB_NS,
RDF_NS, XUL_NS, NC_NS_CMD;
/xpfe/components/bookmarks/resources/bookmarks.js, line 81 -- XUL_NS =
"http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";

All others are [... = document.createElementNS(XUL_NS, "...");]
}

I tried to guess a little, but didn't find out;
additionally, having seen the error only once in v1.3a is not as helpful now
that we have v1.6a :-<

Questions:
*Is there a way to "merge" the two 'const' declarations ?
*Is there a way to improve the 'var' in the .js, as a comment suggests to look
for ? moving to 'const', moving elsewhere, merging ???

Comment 7

[[e-mail]]

Comment on attachment 135203 [details] [diff] [review]
(Part C) <no_space.js> cleanup [File removed]

Although to be fair this patch is of no relevance to the bug and is hardly
worth it as it is, perhaps if you had converted the window to a dialog as
well...

Comment 8

[[e-mail]]

Comment on attachment 135198 [details] [diff] [review]
(Part B) <toolbar.xml> cleanup
[Moved to bug 225328]

I'm sure that adding throw e; must be wrong. This XUL_NS is local to the method
so still not relavent to this bug.

Comment 9

[[e-mail]]

OK, so editorUtilities.js says redeclaration of const XUL_NS and the only other
declaration of XUL_NS is scoped in toolbar.xml, the only possible reason is that
someone is including editorUtilities.js twice (don't be surprised,
contextHelp.js is included twice by messengercompose.xul).

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 135203 [details] [diff] [review]
(Part C) <no_space.js> cleanup [File removed]


Re comment 7:

I guess this patch doesn't need super-review !?

Could you test it (if any test is needed),
then check it in on my behalf ?

Now that we're at it, setting 'approval1.4.2=?' (only guessing that it could
apply) :-|

Thanks.

(I sent you a personal message to take into account the relevance of such a
patch: please, give me some advice.)

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Why is this necessary for 1.4.2?

Comment 12

[[e-mail]]

/me guesses overenthusiasm on the part of the patch author :-)

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 135203 [details] [diff] [review]
(Part C) <no_space.js> cleanup [File removed]


Re comment 11 (and 12):

Unsetting 'approval1.4.2=?':

"
>> In fact, only really important bug fixes move to 1.4.x.
> I stand corrected.
"

Comment 14

[Serge Gautherie (:sgautherie)]

Current situation:

Part A: I have no more clue at this time...

Part B: I'll file another bug for the follow up.

Part C: Now, let's complete it here.

Comment 15

[Serge Gautherie (:sgautherie)]

Comment on attachment 135203 [details] [diff] [review]
(Part C) <no_space.js> cleanup [File removed]


Setting 'sr=?',

from
<http://www.mozilla.org/hacking/reviewers.html>
"editor: kinmoz@netscape_net, sfraser@a_t".

Comment 16

[Serge Gautherie (:sgautherie)]

Comment on attachment 135203 [details] [diff] [review]
(Part C) <no_space.js> cleanup [File removed]


My mistake: 'sfraser@' would be for Part A !
Changing sr=? to "catch-all: brendan@m_g".

Comment 17

[Serge Gautherie (:sgautherie)]

Comment on attachment 135198 [details] [diff] [review]
(Part B) <toolbar.xml> cleanup
[Moved to bug 225328]


Moving Part B to bug 225328.

Comment 18

[Brendan Eich [:brendan]]

Comment on attachment 135203 [details] [diff] [review]
(Part C) <no_space.js> cleanup [File removed]

rs=[e-mail], you don't need to get sr= for whitespace only fixes. 
Please do use cvs diff -wu to show zero non-whitespace changes.

You might add "diff -pu" or "diff -pu8" to your .cvsrc file, for better diffs
in general.

Comment 19

[Serge Gautherie (:sgautherie)]

Comment on attachment 135203 [details] [diff] [review]
(Part C) <no_space.js> cleanup [File removed]


In the meantime:
1) Neil's patch for bug 225819 made <no_space.*> unnecessary.
2) I found more _code_ cleanup to do on <no_space.js>.

Neil:
Should I make this patch obsolete ? :-<
Do you want to check it in now that it's ready for it ? :->
Should I post a v2 patch ? :-|

Comment 20

[Serge Gautherie (:sgautherie)]

Comment on attachment 135203 [details] [diff] [review]
(Part C) <no_space.js> cleanup [File removed]

Nothing left to fix :->

Comment 21

[Serge Gautherie (:sgautherie)]

Updating:
-(K) helpwanted
+(d.o.) 225819
*(S) New -> WorksForMe, as not reported since v1.3a.