Last Comment Bug 1198412 - Use Array.includes in calendar
: Use Array.includes in calendar
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: General (show other bugs)
: Trunk
: Unspecified Unspecified
-- normal (vote)
: 4.6
Assigned To: Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout)
:
:
Mentors:
Depends on: 1208741
Blocks:
  Show dependency treegraph
 
Reported: 2015-08-25 11:49 PDT by Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout)
Modified: 2015-09-26 06:40 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch, v1 (59.04 KB, patch)
2015-08-25 11:49 PDT, Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout)
philipp: review+
Details | Diff | Splinter Review

Description User image Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) 2015-08-25 11:49:51 PDT
Created attachment 8652479 [details] [diff] [review]
patch, v1

Bug 1070767 enabled Array.includes https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes on all branches. The patch passed mozmill and xpcshell tests locally.
Comment 1 User image Philipp Kewisch [:Fallen] 2015-09-19 02:44:51 PDT
Comment on attachment 8652479 [details] [diff] [review]
patch, v1

Review of attachment 8652479 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp with the following fixes. Too bad the unit tests didn't catch this:

::: calendar/providers/gdata/modules/OAuth2.jsm
@@ +154,5 @@
>                        delete this.window;
>                      },
>  
>                      _checkForRedirect: function(aURL) {
> +                      if (!aURL.startsWith(this._parent.completionURI))

gdata's OAuth2.jsm doesn't shim the array/string methods. Either include the shim or keep this at indexOf.

::: calendar/providers/ics/calICSCalendar.js
@@ +665,5 @@
>  
>          function purgeBackupsByType(files, type) {
>              // filter out backups of the type we care about.
>              var filteredFiles = files.filter(
> +                v => { v.name.includes("calBackupData_"+pseudoID+"_"+type) });

If you use brackets with the arrow function, you'll need to return. I'd also prefer splitting the lines a bit differently:

var filteredFiles = files.filter(v => {
    return v.name.includes("calBackupData_" + pseudoID+"_" + type);
});
Comment 2 User image Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) 2015-09-22 02:35:11 PDT
https://hg.mozilla.org/comm-central/rev/990bb78f9284

Note You need to log in before you can comment on or make changes to this bug.