Closed
Bug 1434007
Opened 5 years ago
Closed 5 years ago
Implement String.prototype.trimStart and String.prototype.trimEnd
Categories
(Core :: JavaScript Engine, enhancement, P3)
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: alex.fdm, Assigned: anba)
References
(Blocks 2 open bugs, )
Details
(Keywords: dev-doc-complete)
Attachments
(2 files, 1 obsolete file)
10.33 KB,
patch
|
evilpie
:
review+
|
Details | Diff | Splinter Review |
4.56 KB,
patch
|
anba
:
review+
|
Details | Diff | Splinter Review |
The string trimming proposal is already at STAGE3, and is due to be approved soon: https://github.com/sebmarkbage/ecmascript-string-left-right-trim Firefox already implements String.prototype.trimLeft and String.prototype.trimRight, but String.prototype.trimStart and String.prototype.trimEnd are missing.
Assignee | ||
Comment 1•5 years ago
|
||
- Renames internal uses of trimLeft/Right with trimStart/End. - Doesn't add String.trimStart/End because we want to remove String generics and adding new ones is kind of counter-productive. :-) - The new trimLeft/End functions are currently restricted to nightly-only as long as they're only at stage 3. While I don't expect any compatibility issues, it's probably safer to avoid letting them ride the trains for a release or two. Tests will be added through bug 1434953.
Attachment #8947818 -
Flags: review?(evilpies)
Comment 2•5 years ago
|
||
Comment on attachment 8947818 [details] [diff] [review] bug1434007.patch Review of attachment 8947818 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsstr.cpp @@ +3325,5 @@ > JS_FN("trim", str_trim, 0,0), > +#ifdef NIGHTLY_BUILD > + JS_FN("trimStart", str_trimStart, 0,0), > + JS_FN("trimEnd", str_trimEnd, 0,0), > +#else I thought this might break JS Xrays, but String doesn't seem to use ClassSpec at all.
Attachment #8947818 -
Flags: review?(evilpies) → review+
Comment 3•5 years ago
|
||
(In reply to André Bargull [:anba] from comment #1) > - The new trimLeft/End functions are currently restricted to nightly-only as > long as they're only at stage 3. While I don't expect any compatibility > issues, it's probably safer to avoid letting them ride the trains for a > release or two. FWIW I think it'd be fine to just ship these: they're simple enough that there's no churn to be expected, and shipping in release browsers is a requirement for advancing to stage 4.
Updated•5 years ago
|
Blocks: es-proposals-stage-3
Updated•5 years ago
|
Keywords: dev-doc-needed
Updated•5 years ago
|
Priority: -- → P3
Assignee | ||
Comment 4•5 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e1bdc2b8cff1c8d600fb5bfc03fae9f70a45b5c
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/mozilla-inbound/rev/7e4872ba5521 Implement String.prototype.trim{Start,End} stage 3 proposal. r=evilpie
Keywords: checkin-needed
Comment 6•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7e4872ba5521
Assignee | ||
Comment 7•5 years ago
|
||
(In reply to Till Schneidereit [:till] from comment #3) > FWIW I think it'd be fine to just ship these: they're simple enough that > there's no churn to be expected, and shipping in release browsers is a > requirement for advancing to stage 4. We didn't see any bug reports mentioning trimStart/trimEnd and V8 also seems to ship trimStart/trimEnd in releases <https://bugs.chromium.org/p/v8/issues/detail?id=6530#c4>. The questions is, do we want to enable it by default for Firefox 60, which means also the next ESR, or do we want to wait for Firefox 61?
Attachment #8954298 -
Flags: review?(till)
Comment 8•5 years ago
|
||
Comment on attachment 8954298 [details] [diff] [review] bug1434007-enable-in-release.patch Review of attachment 8954298 [details] [diff] [review]: ----------------------------------------------------------------- Apologies for the late reply - I was out sick last week. At least that got us around the question whether to ship this in 60 or not, given that it's now too late for that :/
Attachment #8954298 -
Flags: review?(till) → review+
Assignee | ||
Comment 9•5 years ago
|
||
Updated to apply cleanly on inbound, carrying r+.
Attachment #8954298 -
Attachment is obsolete: true
Attachment #8961583 -
Flags: review+
Assignee | ||
Comment 10•5 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=13378bb17341a1505c614448df0221716fb67021
Keywords: leave-open → checkin-needed
Comment 11•5 years ago
|
||
Pushed by csabou@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6771d444d64f Enable String.prototype.trim{Start,End} by default. r=till
Keywords: checkin-needed
Comment 12•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6771d444d64f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment 13•5 years ago
|
||
Announced here: https://developer.mozilla.org/en-US/Firefox/Releases/61#JavaScript Updated: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/trimStart https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/trimEnd Interactive example and compat data are still pending review, but will get done here: https://github.com/mdn/interactive-examples/pull/922 https://github.com/mdn/browser-compat-data/pull/2013 Let me know if this looks OK to you.
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 14•5 years ago
|
||
(In reply to Florian Scholz [:fscholz] (MDN) from comment #13) > Let me know if this looks OK to you. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/trimEnd has a typo in its first line: "trimend" instead of "trimEnd". Otherwise lgtm!
Comment 15•5 years ago
|
||
Thanks, fixed :)
You need to log in
before you can comment on or make changes to this bug.
Description
•