Remove nonstandard RegExp.multiline global switch

RESOLVED FIXED in Firefox 48

Status

()

Core
JavaScript Engine
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: Claude Pache, Assigned: arai)

Tracking

(Blocks: 1 bug, {dev-doc-complete, site-compat})

unspecified
mozilla48
dev-doc-complete, site-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [DocArea=JS])

Attachments

(13 attachments, 3 obsolete attachments)

8.13 KB, patch
till
: review+
Details | Diff | Splinter Review
20.03 KB, patch
till
: review+
Details | Diff | Splinter Review
4.13 KB, patch
till
: review+
Details | Diff | Splinter Review
8.66 KB, patch
till
: review+
Details | Diff | Splinter Review
5.24 KB, patch
till
: review+
Details | Diff | Splinter Review
8.31 KB, patch
till
: review+
Details | Diff | Splinter Review
1.62 KB, patch
till
: review+
Details | Diff | Splinter Review
1.91 KB, patch
till
: review+
Details | Diff | Splinter Review
3.58 KB, patch
till
: review+
Details | Diff | Splinter Review
4.64 KB, patch
till
: review+
Details | Diff | Splinter Review
3.52 KB, patch
till
: review+
Details | Diff | Splinter Review
1.95 KB, patch
till
: review+
Details | Diff | Splinter Review
3.86 KB, patch
till
: review+
Details | Diff | Splinter Review
(Reporter)

Description

2 years ago
Test case:

(function() { 
    RegExp.multiline = true; 
    if (!RegExp.multiline) 
        return 'RegExp.multiline cannot be set to true...';
    if (/foo/.multiline)
        return "RegExp.multiline forces multiline flag on new RegExp :-(";
    else
        return "RegExp.multiline has no effect :-)";
})()

Safari and Chrome ignore that misfeature, so it's not needed for web compatibility.
(Reporter)

Updated

2 years ago
Blocks: 1103158
(Assignee)

Comment 1

2 years ago
https://dxr.mozilla.org/mozilla-central/search?q=RegExp.multiline&case=true&=mozilla-central&redirect=true
It's not also used in m-c.

Can anyone search in AMO MXR?
It's used in 5 addons:

https://addons.mozilla.org/en-US/seamonkey/addon/enigmail/
https://addons.mozilla.org/en-US/firefox/addon/xinha-here/
https://addons.mozilla.org/en-US/firefox/addon/searchwith/
https://addons.mozilla.org/en-US/firefox/addon/save-images/ (Help text only, not functionally relevant.)
https://addons.mozilla.org/en-US/thunderbird/addon/dooth/?src=search (Bundles enigmail and thus has the same uses of RegExp.multiline.)

Only the enigmail use seems worrying to me. From a glance at the code, it seems like removing RegExp.multiline *might* silently break encryption. Kinda unfortunate, really. The uses are in these files:
http://sourceforge.net/p/enigmail/source/ci/master/tree/ui/content/enigmailMsgComposeOverlay.js
http://sourceforge.net/p/enigmail/source/ci/master/tree/package/decryption.jsm

Seems easy enough to fix.
(Assignee)

Comment 3

2 years ago
Created attachment 8681068 [details] [diff] [review]
Draft patch for Enigmail

Any Enigmail developer available in bmo?
(Assignee)

Comment 4

2 years ago
Created attachment 8681070 [details] [diff] [review]
Draft patch for Enigmail

Just noticed they have git repo.
Will post there too.
Attachment #8681068 - Attachment is obsolete: true
(Assignee)

Comment 5

2 years ago
Opened https://sourceforge.net/p/enigmail/bugs/532/
(Reporter)

Comment 6

2 years ago
Remind that RegExp.multiline has a synonym named "$*":

```js
RegExp['$*'] = true
RegExp.multiline // true
```
(Assignee)

Comment 7

2 years ago
Thank you!

http://mxr.mozilla.org/mozilla-central/search?string=RegExp\[.\%24\*.&regexp=1&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
It's also not used in m-c
Doesn't seem to be used in addons, either. I'm not surprised, because that is just a travesty.
(Assignee)

Comment 9

2 years ago
Comment on attachment 8681070 [details] [diff] [review]
Draft patch for Enigmail

use in enigmail is fixed in trunk :)
http://sourceforge.net/p/enigmail/source/ci/aa50fc73f9f3df05f38280c2a895382f98464316/
Attachment #8681070 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Depends on: 1220457
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
(Assignee)

Comment 10

2 years ago
Created attachment 8691925 [details]
WIP patches

Here's draft patches to remove RegExp.multiline and related things.
Patch in bug 1220457 should be backed out before applying them.

Updated

2 years ago
Keywords: site-compat
(Assignee)

Comment 11

a year ago
There's Enigmail 1.9b1 with the fix.
https://addons.mozilla.org/en-US/thunderbird/addon/enigmail/

maybe we could remove RegExp.multiline in next cycle
(Assignee)

Comment 12

a year ago
Enigmail 1.9 with the fix has been released.

will prepare patches.
(Assignee)

Comment 13

a year ago
try is running https://treeherder.mozilla.org/#/jobs?repo=try&revision=2821ae9a654d
(Assignee)

Comment 14

a year ago
Created attachment 8729119 [details] [diff] [review]
Part 0: Remove RegExp.multiline warning.

Just backs out bug 1220457 patch.
Assignee: nobody → arai.unmht
Attachment #8691925 - Attachment is obsolete: true
Attachment #8729119 - Flags: review?(till)
(Assignee)

Comment 15

a year ago
Created attachment 8729122 [details] [diff] [review]
Part 1: Remove RegExp.multiline accessor.

Removed RegExp.multiline and RegExp.$* accessors, and tests for them.
Attachment #8729122 - Flags: review?(till)
(Assignee)

Comment 16

a year ago
Created attachment 8729123 [details] [diff] [review]
Part 2: Remove RegExpStaticsUse parameter from RegExpInitialize.
Attachment #8729123 - Flags: review?(till)
(Assignee)

Comment 17

a year ago
Created attachment 8729124 [details] [diff] [review]
Part 3: Rename Self-hosting regexp_construct_no_statics to regexp_construct.
Attachment #8729124 - Flags: review?(till)
(Assignee)

Comment 18

a year ago
Created attachment 8729127 [details] [diff] [review]
Part 4: Remove RegExpStatics* parameter from RegExpObject::create.
Attachment #8729127 - Flags: review?(till)
(Assignee)

Comment 19

a year ago
Created attachment 8729130 [details] [diff] [review]
Part 5: Remove RegExpObject::createNoStatics.

Now RegExpObject::create and RegExpObject::createNoStatics are same, so merged to RegExpObject::create.
Attachment #8729130 - Flags: review?(till)
(Assignee)

Comment 20

a year ago
Created attachment 8729133 [details] [diff] [review]
Part 6: Remove multiline parameter from RegExpStatics::reset.
Attachment #8729133 - Flags: review?(till)
(Assignee)

Comment 21

a year ago
Created attachment 8729136 [details] [diff] [review]
Part 7: Remove RegExpStatics::multiline and RegExpStatics::setMultiline.
Attachment #8729136 - Flags: review?(till)
(Assignee)

Comment 22

a year ago
Created attachment 8729137 [details] [diff] [review]
Part 8: Remove RegExpStatics::getFlags and RegExpStatics::flags.
Attachment #8729137 - Flags: review?(till)
(Assignee)

Comment 23

a year ago
Created attachment 8729139 [details] [diff] [review]
Part 9: Remove OBJECT_FLAG_REGEXP_FLAGS_SET flag.
Attachment #8729139 - Flags: review?(till)
(Assignee)

Comment 24

a year ago
Created attachment 8729141 [details] [diff] [review]
Part 10: Remove HandleObject parameter from JS_NewRegExpObject and JS_NewUCRegExpObject.
Attachment #8729141 - Flags: review?(till)
(Assignee)

Comment 25

a year ago
Created attachment 8729143 [details] [diff] [review]
Part 11: Remove multiline parameter from JS_SetRegExpInput.
Attachment #8729143 - Flags: review?(till)
(Assignee)

Comment 26

a year ago
Created attachment 8729145 [details] [diff] [review]
Part 12: Remove JS_NewRegExpObjectNoStatics and JS_NewUCRegExpObjectNoStatics.

Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c0f23e18a883
Attachment #8729145 - Flags: review?(till)
Comment on attachment 8729119 [details] [diff] [review]
Part 0: Remove RegExp.multiline warning.

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

r=me
Attachment #8729119 - Flags: review?(till) → review+
Comment on attachment 8729122 [details] [diff] [review]
Part 1: Remove RegExp.multiline accessor.

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

\o/, one weird corner slightly cleaned up!
Attachment #8729122 - Flags: review?(till) → review+
Comment on attachment 8729123 [details] [diff] [review]
Part 2: Remove RegExpStaticsUse parameter from RegExpInitialize.

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

r=me
Attachment #8729123 - Flags: review?(till) → review+
Comment on attachment 8729124 [details] [diff] [review]
Part 3: Rename Self-hosting regexp_construct_no_statics to regexp_construct.

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

r=me

::: js/src/builtin/RegExp.h
@@ +75,5 @@
>  extern bool
>  regexp_test_no_statics(JSContext* cx, unsigned argc, Value* vp);
>  
>  /*
> + * Behaves like RegExp(string) or RegExp(string, string), for Self-hosted JS.

nit: "self-hosted" (no capitalization)
Attachment #8729124 - Flags: review?(till) → review+
Comment on attachment 8729127 [details] [diff] [review]
Part 4: Remove RegExpStatics* parameter from RegExpObject::create.

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

Nice. r=me
Attachment #8729127 - Flags: review?(till) → review+
Comment on attachment 8729130 [details] [diff] [review]
Part 5: Remove RegExpObject::createNoStatics.

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

We should be able to remove the related JSAPI functions, too, right? JS_NewRegExpNoStatics doesn't seem to be used at all anyway, and the UC version should be replaceable with the normal version, right? Also JS_ClearRegExpStatics
Attachment #8729130 - Flags: review?(till) → review+
Comment on attachment 8729133 [details] [diff] [review]
Part 6: Remove multiline parameter from RegExpStatics::reset.

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

Hmm, I guess what I just said isn't true because the RegExpStatics are still used, so ignore that.
Attachment #8729133 - Flags: review?(till) → review+
Comment on attachment 8729136 [details] [diff] [review]
Part 7: Remove RegExpStatics::multiline and RegExpStatics::setMultiline.

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

r=me
Attachment #8729136 - Flags: review?(till) → review+
Comment on attachment 8729145 [details] [diff] [review]
Part 12: Remove JS_NewRegExpObjectNoStatics and JS_NewUCRegExpObjectNoStatics.

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

Oh, except you are actually removing them!
Attachment #8729145 - Flags: review?(till) → review+
Comment on attachment 8729137 [details] [diff] [review]
Part 8: Remove RegExpStatics::getFlags and RegExpStatics::flags.

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

r=me
Attachment #8729137 - Flags: review?(till) → review+
Comment on attachment 8729143 [details] [diff] [review]
Part 11: Remove multiline parameter from JS_SetRegExpInput.

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

r=me
Attachment #8729143 - Flags: review?(till) → review+
Comment on attachment 8729141 [details] [diff] [review]
Part 10: Remove HandleObject parameter from JS_NewRegExpObject and JS_NewUCRegExpObject.

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

r=me
Attachment #8729141 - Flags: review?(till) → review+
Comment on attachment 8729139 [details] [diff] [review]
Part 9: Remove OBJECT_FLAG_REGEXP_FLAGS_SET flag.

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

r=me
Attachment #8729139 - Flags: review?(till) → review+
(Assignee)

Comment 40

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/75826602a7885b740b13a02281f0e6462345baa0
Bug 1219757 - Part 0: Remove RegExp.multiline warning. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/e5ae8475e40e2dd74033f3163e42811e1fc6de88
Bug 1219757 - Part 1: Remove RegExp.multiline accessor. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/c158afbc85abaa8352e8341fd197023073157d45
Bug 1219757 - Part 2: Remove RegExpStaticsUse parameter from RegExpInitialize. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/7bb43f9d76affc52ca45036424da6f33107929d5
Bug 1219757 - Part 3: Rename Self-hosting regexp_construct_no_statics to regexp_construct. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ba888fc9595655b04776d6e693f79943290747c
Bug 1219757 - Part 4: Remove RegExpStatics* parameter from RegExpObject::create. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/1090265b5b38d364569b950444194e4084908117
Bug 1219757 - Part 5: Remove RegExpObject::createNoStatics. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e6d7810b79a6c57538f3241c23896c2dccf5b68
Bug 1219757 - Part 6: Remove multiline parameter from RegExpStatics::reset. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca7fffec2dddbc955428a27379cfc344520bb4fd
Bug 1219757 - Part 7: Remove RegExpStatics::multiline and RegExpStatics::setMultiline. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/485ed8b35eac2d1d15064163e07600ccafc8d239
Bug 1219757 - Part 8: Remove RegExpStatics::getFlags and RegExpStatics::flags. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ed1480aac04f99f41988a1ee16615328cc6d350
Bug 1219757 - Part 9: Remove OBJECT_FLAG_REGEXP_FLAGS_SET flag. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/c11dcc75f7a47792d391f447ab77ebb40894c3ff
Bug 1219757 - Part 10: Remove HandleObject parameter from JS_NewRegExpObject and JS_NewUCRegExpObject. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/79925a383bbc06be1a47c730f2d094a5c2a82d07
Bug 1219757 - Part 11: Remove multiline parameter from JS_SetRegExpInput. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/604a180b6cc0e101eef1f974356b613aa0a40146
Bug 1219757 - Part 12: Remove JS_NewRegExpObjectNoStatics and JS_NewUCRegExpObjectNoStatics. r=till

Comment 41

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/75826602a788
https://hg.mozilla.org/mozilla-central/rev/e5ae8475e40e
https://hg.mozilla.org/mozilla-central/rev/c158afbc85ab
https://hg.mozilla.org/mozilla-central/rev/7bb43f9d76af
https://hg.mozilla.org/mozilla-central/rev/3ba888fc9595
https://hg.mozilla.org/mozilla-central/rev/1090265b5b38
https://hg.mozilla.org/mozilla-central/rev/1e6d7810b79a
https://hg.mozilla.org/mozilla-central/rev/ca7fffec2ddd
https://hg.mozilla.org/mozilla-central/rev/485ed8b35eac
https://hg.mozilla.org/mozilla-central/rev/8ed1480aac04
https://hg.mozilla.org/mozilla-central/rev/c11dcc75f7a4
https://hg.mozilla.org/mozilla-central/rev/79925a383bbc
https://hg.mozilla.org/mozilla-central/rev/604a180b6cc0
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/regexp-multiline-global-switch-has-been-removed/
Mentioned on
https://developer.mozilla.org/en-US/Firefox/Releases/48#JavaScript
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Deprecated_and_obsolete_features#RegExp_properties
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/multiline#Compatibility_notes
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.