Closed Bug 1361856 Opened 5 years ago Closed 2 years ago

Implement RegExp s (dotAll) flag

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla78
Webcompat Priority ?
Tracking Status
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- fixed

People

(Reporter: leonardo.balter, Assigned: iain)

References

()

Details

(4 keywords)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.110 Safari/537.36

Steps to reproduce:

TC39's stage 3 proposal

https://github.com/tc39/proposal-regexp-dotall-flag

Test262 files under the `regexp-dotall` features flag:

test/annexB/built-ins/RegExp/prototype/flags/order-after-compile.js
test/built-ins/RegExp/dotall/with-dotall-unicode.js
test/built-ins/RegExp/dotall/with-dotall.js
test/built-ins/RegExp/duplicate-flags.js
test/built-ins/RegExp/prototype/dotAll/length.js
test/built-ins/RegExp/prototype/dotAll/name.js
test/built-ins/RegExp/prototype/dotAll/prop-desc.js
test/built-ins/RegExp/prototype/dotAll/this-val-invalid-obj.js
test/built-ins/RegExp/prototype/dotAll/this-val-non-obj.js
test/built-ins/RegExp/prototype/dotAll/this-val-regexp-prototype.js
test/built-ins/RegExp/prototype/dotAll/this-val-regexp.js
test/built-ins/RegExp/prototype/flags/order.js
test/built-ins/RegExp/prototype/flags/s.js
I'm also adding these tests to the test/jstests.list file
The skip list is here: https://bugzilla.mozilla.org/show_bug.cgi?id=1362169
Depends on: 1367105
Keywords: triage-deferred
Priority: -- → P3
This feature was approved for ES2018 and is now standard.  It is already supported by current Chrome and by Safari TP.
Status: UNCONFIRMED → NEW
Ever confirmed: true
No longer blocks: es-proposals-stage-4
This was supported in Chrome 64 and now on Safari 11.1.  Maybe some "parity" tags should be added here.
Type: defect → enhancement
Duplicate of this bug: 1579867

(In reply to David Spector from comment #7)

but I see no reason to delay further, as the documentation (see citation in my bug report) already states that the s flag exists. It can't be very difficult to implement, in my opinion, from a technical point of view.

I believe that Firefox documentation and software should be kept in synchronization somehow. Perhaps you don't store the documentation in the same git as the software? You might consider coordinating them better, as I've seen other examples of inconsistency.

Please note that MDN web docs is a browser comprehensive wiki documenting the web standards, it's not (just) a "Firefox documentation". And the browser compatibility section of the page you referenced in bug 1579867 clearly states that the "dotAll" feature covered here is not supported by Firefox yet.

As far as I can tell, the relevant code for this touches the files at https://searchfox.org/mozilla-central/search?q=&case=false&regexp=false&path=js%2F**%2FRegExp*, presumably mainly https://searchfox.org/mozilla-central/source/js/src/builtin/RegExp.cpp.

Having said that, I am far from being able to implement this myself.

Sebastian

(In reply to github from comment #10)

In 2018, the dotall flag was added to the specification. Here we are approaching 2020 and Firefox still hasn't implemented it. In what year will Mozilla implement the 2018 specification?

Please stop asking this question in every single regular expression related bug, you already got an answer in bug 1362154.

(In reply to David Spector from comment #13)

(In reply to Tom Schuster [:evilpie] from comment #11)

Please stop asking this question in every single regular expression related bug, you already got an answer in bug 1362154.

  1. To whom are you directing this statement? To me? Someone else?
  2. What answer are you referring to in #1362154? It is not obvious.

Sending brief, ambiguous, and nasty comments is not helpful to fixing old bugs.

Sorry, this is an answer to another comment that is now hidden. Ian is actively working on updating our copy of the v8 regular expression code that should enable us to run all new regular expression features from ES2018 (bug 1362154 comment 22). You can follow this work in the meta bug 1367105.

This patch adds the boilerplate necessary to support a new regexp flag. Externally visible changes (parsing the flag, the dotAll property on the prototype) are guarded behind ENABLE_NEW_REGEXP.

The actual implementation of dotAll comes for free with the fresh import of irregexp (bug 1367105).

There is one test (tests/non262/RegExp/prototype.js) that needs to be updated when this is turned on to add "dotAll" to the list of expected properties on the RegExp prototype. I will attach that change to my patch that flips ENABLE_NEW_REGEXP to be on by default.

Assignee: nobody → iireland
Status: NEW → ASSIGNED
Pushed by iireland@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8a017673c25b
Add dotAll to RegExpFlags r=jwalden
https://hg.mozilla.org/integration/autoland/rev/597e083be9d4
Use SM RegExpFlags inside irregexp r=jwalden

I missed one externally visible change: RegExp.prototype.flags can be called on non-regexp objects, which can have dotAll properties defined. I could have sworn that this code had been through several try builds, but I guess I must have changed RegExpFlagsGetter afterwards. I've uploaded a fix and kicked off a precautionary try build. I will reland once the try build completes.

The jsreftest failures are the same testcase.

Flags: needinfo?(iireland)
Pushed by iireland@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f288e81410d
Add dotAll to RegExpFlags r=jwalden
https://hg.mozilla.org/integration/autoland/rev/736f7749e6a7
Use SM RegExpFlags inside irregexp r=jwalden
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76

Since this is now closed, shouldn't today's nightly (mar/25/2020) be passing the test at http://kangax.github.io/compat-table/es2016plus/ then?

No, this depends on bug 1367105. There's no more dotAll-specific work to be done, but we won't have dotAll support until that lands.

Resolution: FIXED → MOVED
Status: RESOLVED → REOPENED
Resolution: MOVED → ---
Webcompat Priority: --- → ?
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED

Fixed by the landing of bug 1634135.

Target Milestone: mozilla76 → mozilla78
You need to log in before you can comment on or make changes to this bug.