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

RESOLVED FIXED in seamonkey2.1final

Status

defect
RESOLVED FIXED
19 years ago
8 years ago

People

(Reporter: timeless, Assigned: ewong)

Tracking

Trunk
seamonkey2.1final

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(1 attachment, 3 obsolete attachments)

Please move
  <script language="JavaScript">
  <![CDATA[
...
  ]]>
  </script>
into a .js file.
See related bugs 56365, 56368, 56370, 56372, and 56374.
Why create more files? I don't see this as a bug.
QA Contact: esther → stephend
Product: Browser → Seamonkey
Assignee: scottputterman → mail
Priority: P3 → --
QA Contact: stephend → search
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/
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → EXPIRED
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/
Surprisingly enough, just the passage of time doesn't make it right to include script in XUL.
Status: RESOLVED → REOPENED
Resolution: EXPIRED → ---
Assignee: mail → nobody
QA Contact: search → message-display
Status: REOPENED → NEW
Taking this for a spin.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #523980 - Flags: review?(mnyromyr)
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.
Attachment #523980 - Flags: review?(mnyromyr) → review-
Attachment #523980 - Attachment is obsolete: true
Attachment #526724 - Flags: review?(mnyromyr)
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.
Attachment #526724 - Flags: review?(mnyromyr) → review-
Attachment #526724 - Attachment is obsolete: true
Attachment #526968 - Flags: review?(mnyromyr)
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.
Attachment #526968 - Flags: superreview+
Attachment #526968 - Flags: review?(mnyromyr)
Attachment #526968 - Flags: review+
Fixed obvious mistake in v3.
Attachment #526968 - Attachment is obsolete: true
Attachment #527266 - Flags: superreview+
Attachment #527266 - Flags: review+
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Closed: 11 years ago8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1final
You need to log in before you can comment on or make changes to this bug.