Use String#slice instead of String#substr or String#substring
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 3 Sep 2014 15:19:41 +0000 (17:19 +0200)
committerOri.livneh <ori@wikimedia.org>
Wed, 3 Sep 2014 21:33:06 +0000 (21:33 +0000)
commitc71818346673c91987c201543788d802f346cad5
treeac0220c159347a38a3e2ff071932eb7a871bbf66
parent40dc9713f1c9a91aacbf457b5632db2ac4ea1b15
Use String#slice instead of String#substr or String#substring

Quite a few reasons:

* There is a bug in IE 8 and below where the startIndex argument
  does not support negative values, contrary to the ECMAScript
  spec and implementations in other browsers.
  IE8:
    'faux'.substr( -1 ); // "faux"
  Standards:
    'faux'.substr( -1 ); // "x"

  Code written for ES5 (and using the es5-shim) works as expected
  since the shim repairs this method.

* String#substr and String#substring both exist but have
  different signatures which are easily mixed up.

  String.prototype.substr( start [, length] )
  > Supports negative start, but not in IE8 and below.
  > Takes length, *not* second index. E.g. `substr( 2, 3 )`
    returns `slice( 2, 5 )`.

  String.prototype.substring( indexA [, indexB] )
  > Doesn't support negative indices.
  > If indexA is larger than indexB, they are silently swapped!

  String.prototype.slice( start [, end] )
  > Supports negative indices.

    'faux'.substr( 0, 2 );     // "fa"
    'faux'.substring( 0, 2 );  // "fa"
    'faux'.slice( 0, 2 );      // "fa"

    'faux'.substr( -2 );       // "ux"
    'faux'.substring( -2 );    // "faux"
    'faux'.slice( -2 );        // "ux"

    'faux'.substr( 1, 2 );     // "au"
    'faux'.substring( 1, 2 );  // "a"
    'faux'.slice( 1, 2 );      // "a"

    'faux'.substr( 1, -1 );    // ""
    'faux'.substring( 1, -1 ); // "f"
    'faux'.slice( 1, -1 );     // "au"

    'faux'.substr( 2, 1 );     // "u"
    'faux'.substring( 2, 1 );  // "a"
    'faux'.slice( 2, 1 );      // ""

* String#slice works the same way as Array#slice and slice
  methods elsewhere (jQuery, PHP, ..).

* Also simplify calls:
  - Omit second argument where it explicitly calculated the length
    and passed it as is (default behaviour)
  - Pass negative values instead of length - x.
  - Use chatAt to extract a single character.

* Change:
  - Replace all uses of substring() with slice().
  - Replace all uses of substr() with slice() where only one
    parameter is passed or where the first parameter is 0.
  - Using substr()'s unique behaviour (length instead of endIndex)
    is fine, though there's only one instances of that, in
    mediawiki.jqueryMsg.js

Change-Id: I9b6dd682a64fa28c7ea0da1846ccd3b42f9430cf
26 files changed:
mw-config/config.js
resources/src/jquery/jquery.autoEllipsis.js
resources/src/jquery/jquery.byteLimit.js
resources/src/jquery/jquery.farbtastic.js
resources/src/jquery/jquery.hidpi.js
resources/src/jquery/jquery.mwExtension.js
resources/src/jquery/jquery.textSelection.js
resources/src/mediawiki.language/languages/fi.js
resources/src/mediawiki.language/languages/he.js
resources/src/mediawiki.language/languages/hy.js
resources/src/mediawiki.language/languages/os.js
resources/src/mediawiki.language/languages/ru.js
resources/src/mediawiki.language/languages/uk.js
resources/src/mediawiki.language/mediawiki.language.js
resources/src/mediawiki.language/mediawiki.language.numbers.js
resources/src/mediawiki.legacy/upload.js
resources/src/mediawiki.special/mediawiki.special.search.js
resources/src/mediawiki/mediawiki.Title.js
resources/src/mediawiki/mediawiki.debug.js
resources/src/mediawiki/mediawiki.debug.profile.js
resources/src/mediawiki/mediawiki.inspect.js
resources/src/mediawiki/mediawiki.jqueryMsg.js
resources/src/mediawiki/mediawiki.js
resources/src/mediawiki/mediawiki.user.js
tests/qunit/suites/resources/jquery/jquery.autoEllipsis.test.js
tests/qunit/suites/resources/mediawiki/mediawiki.test.js