463060 - Clean-up and move clipboard.js

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Martin Schröder [:mschroeder]]

Attachment: Patch v1

* Clean-up of clipboard.js including removal of Mozilla 1.8 compatibility
* Move resources/content/clipboard.js to base/content/calendar-clipboard.js
* Adjust jar.mn files for moved file

Comment 1

[Simon Paquet [:sipaq]]

Comment on attachment 346268 [details] [diff] [review]
Patch v1

>diff --git a/calendar/resources/content/clipboard.js b/calendar/base/content/calendar-clipboard.js
>rename from calendar/resources/content/clipboard.js
>rename to calendar/base/content/calendar-clipboard.js
>--- a/calendar/resources/content/clipboard.js
>+++ b/calendar/base/content/calendar-clipboard.js
>@@ -13,264 +13,209 @@
>  *
>  * The Original Code is Mozilla Calendar code.
>  *
>  * The Initial Developer of the Original Code is
>  * ArentJan Banck <[e-mail]>.
>  * Portions created by the Initial Developer are Copyright (C) 2002
>  * the Initial Developer. All Rights Reserved.
>  *
>- * Contributor(s): ArentJan Banck <[e-mail]>
>- *                 Joey Minta <[e-mail]>
>- *                 Philipp Kewisch <[e-mail]>
>+ * Contributor(s):
>+ *   ArentJan Banck <[e-mail]>

Why are we naming ArentJan here, when he is already the initial developer? That seems to be not necessary, right?

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 346268 [details] [diff] [review]
Patch v1

>+function canPaste() {

>+    var flavors = ["text/calendar", "text/unicode"];
This can be const

>+    return getClipboard().hasDataMatchingFlavors(flavors, flavors.length,
>+                                                 Components.interfaces.nsIClipboard.kGlobalClipboard);
If you wrap parameters, wrap em all

>+    var trans = Components.classes["@mozilla.org/widget/transferable;1"]
>+                          .createInstance(Components.interfaces.nsITransferable);
Please use let instead of var in this patch where appropriate.


>                     case "VEVENT":
>+                        var event = createEvent();
In light of migration to calUtils.jsm, please use cal.createEvent() instead. The same goes for createTodo() and any other calUtils.js functions you find.

>             var earliestDate = null;
>             for each(item in items) {
Was item defined beforehand? If not, please use for each (let ...) (mind the missing space)


r=philipp

Comment 3

[Martin Schröder [:mschroeder]]

Attachment: Patch v2 (w/o actual file move)

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 346437 [details] [diff] [review]
Patch v2 (w/o actual file move)

Looks good, r=philipp

Comment 5

[Martin Schröder [:mschroeder]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/020882e3433b>

-> FIXED

Comment 6

[Stefan Sitter [:ssitter]]

This checkin most probably regressed Bug 463784.