56365 - mailOverlay.xul abuses <script>...</script>

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[timeless]

Please move
  <script language="JavaScript">
  <![CDATA[
...
  ]]>
  </script>
into a .js file.

Comment 1

[Stephen P. Morse]

See related bugs 56365, 56368, 56370, 56372, and 56374.

Comment 2

[Håkan Waara]

Why create more files? I don't see this as a bug.

Comment 3

[Arun Prasannan (polar)]

This bug is being marked EXPIRED as it has seen no activity in a very long time.

If you think that the issue reported might still be relevant, please test with a recent release of SeaMonkey and if the problem persists feel free to re-open the report. Thank you.

http://www.seamonkey-project.org/

Comment 4

[Arun Prasannan (polar)]

This bug is being marked EXPIRED as it has seen no activity in a very long time.

If you think that the issue reported might still be relevant, please test with a recent release of SeaMonkey and if the problem persists feel free to re-open the report. Thank you.

http://www.seamonkey-project.org/

Comment 5

[Phil Ringnalda (:philor)]

Surprisingly enough, just the passage of time doesn't make it right to include script in XUL.

Comment 6

[Edmund Wong (:ewong)]

Taking this for a spin.

Comment 7

[Edmund Wong (:ewong)]

Attachment: Moved the scripting into a new js file.

Comment 8

[Karsten Düsterloh]

Comment on attachment 523980 [details] [diff] [review]
Moved the scripting into a new js file.

>+++ b/suite/mailnews/openMsgDialog.js

What's the reason not to name it mailOverlay.js?
Usually a .js file included by just one .xul file gets the same base name.

>+ * The Original Code is Thunderbird Mail Client.

I very much doubt that. ;-)

>+ * The Initial Developer of the Original Code is
>+ * the Mozilla Foundation.
>+ * Portions created by the Initial Developer are Copyright (C) 2009
>+ * the Initial Developer. All Rights Reserved.

You may want rethink those as well. ;-)

>+function goOpenNewMessage()
>+{
…
>+  var msgComposeService = Components.classes["@mozilla.org/messengercompose;1"].getService();
>+  msgComposeService = msgComposeService.QueryInterface(Components.interfaces.nsIMsgComposeService);

While touching this, you could as well move the QI into the getService call.

Comment 9

[Edmund Wong (:ewong)]

Attachment: Moved JS code to mailOverlay.js

Comment 10

[Karsten Düsterloh]

Comment on attachment 526724 [details] [diff] [review]
Moved JS code to mailOverlay.js

Just two nits:

>+ * The Original Code is SeaMonkey code.
>+ *
>+ * The Initial Developer of the Original Code is
>+ * the Mozilla Foundation.
>+ *
>+ * Portions created by the Initial Developer are Copyright (C) 2011
>+ * the Initial Developer. All Rights Reserved.

According to <http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/base/resources/content/mailOverlay.xul&rev=1.22&mark=10-40>, the code was written initially in 2000-2003, so just use that range instead of 2011.

>+  var msgComposeService =
>+    Components.classes["@mozilla.org/messengercompose;1"]
>+              .getService(Components.interfaces.nsIMsgComposeService);
>+
>+  msgComposeService.OpenComposeWindow(null, null, null,
>+                                      Components.interfaces.nsIMsgCompType.New,
>+                                      Components.interfaces.nsIMsgCompFormat.Default,
>+                                      null, null);

Contemplating over this a bit I think we don't need the variable msgComposeService at all, just take care of the indentation.

Comment 11

[Edmund Wong (:ewong)]

Attachment: Moved JS code to mailOverlay.js (v3)

Comment 12

[Karsten Düsterloh]

Comment on attachment 526968 [details] [diff] [review]
Moved JS code to mailOverlay.js (v3)

>+  Components.classes["@mozilla.org/messengercompose;1"]
>+            .getService(Components.interfaces.nsIMsgComposeService);
>+            .OpenComposeWindow(null, null, null,
>+                               Components.interfaces.nsIMsgCompType.New,
>+                               Components.interfaces.nsIMsgCompFormat.Default,
>+                               null, null);

I know, some changes seem dead simple, but I'd really suggest applying patches and at least checking the JS error console before attaching them. :-P

r/moa=me with the obvious syntax error fixed.

Comment 13

[Edmund Wong (:ewong)]

Attachment: Moved JS code to mailOverlay.js (v4)

Fixed obvious mistake in v3.

Comment 14

[Philip Chee]

I fixed your DOS line endings before pushing!

Pushed:
http://hg.mozilla.org/comm-central/rev/62b048f67953
http://hg.mozilla.org/releases/comm-2.0/rev/7ca68abfacca