Opened on 02/18/2019 at 06:11:34 PM
Closed on 02/21/2019 at 08:15:50 PM
Last modified on 07/25/2019 at 02:20:02 PM
#7303 closed change (fixed)
Deprecate the use of String.prototype.substr()
| Reported by: | hfiguiere | Assignee: | hfiguiere |
|---|---|---|---|
| Priority: | P4 | Milestone: | |
| Module: | Core | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by hfiguiere)
Background
According to MDN:
Warning: Although String.prototype.substr(…) is not strictly deprecated (as in "removed from the Web standards"), it is considered a legacy function and should be avoided when possible. It is not part of the core JavaScript language and may be removed in the future. If at all possible, use the substring() method instead.
We should move to towards using String.prototype.substring() everywhere
What to change
Replace substr() with substring(). One caveat: the optional second argument in not the length but instead the index of the first character to exclude, so attention must be paid to that when changing.
Hint for tester
Changes are small and spread out in the code base. The unit tests did pass. Interesting errors with string truncated or at the wrong offset could be symptoms of errors in that changeset.
Attachments (0)
Change History (6)
comment:1 Changed on 02/19/2019 at 04:47:39 PM by hfiguiere
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:2 Changed on 02/19/2019 at 04:47:49 PM by hfiguiere
- Owner set to hfiguiere
comment:3 Changed on 02/21/2019 at 07:55:01 PM by mjethani
- Priority changed from Unknown to P4
- Ready set
comment:4 Changed on 02/21/2019 at 08:11:42 PM by abpbot
comment:5 Changed on 02/21/2019 at 08:15:50 PM by hfiguiere
- Description modified (diff)
- Resolution set to fixed
- Status changed from reviewing to closed
Two locations have been left out of the patch deliberately as they are in hot path and substr() is apparently (a very little bit) faster on V8.
comment:6 Changed on 07/25/2019 at 02:20:02 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Done. Does not look to have caused any regessions.
ABP 0.9.15.2340
Microsoft Edge 44.17763.1.0 / Windows 10 1809
ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
Firefox Mobile 68.0 / Android 7.2.2
A commit referencing this issue has landed:
Issue 7303 - Deprecate the use of String.substr()