Phabricator will be unavailable due to database maintenance from 14:00 UTC until 18:00 UTC on Saturday, October 13, 2018.
Bugzilla will remain up during this time. All users have been logged out of Bugzilla

Remove nonstandard RegExp.multiline global switch

RESOLVED FIXED in Firefox 48

Status

()

RESOLVED FIXED
3 years ago
3 years 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

3 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

3 years ago
Blocks: 1103158
(Assignee)

Comment 1

3 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

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

Any Enigmail developer available in bmo?
(Assignee)

Comment 4

3 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
(Reporter)

Comment 6

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

```js
RegExp['$*'] = true
RegExp.multiline // true
```
Doesn't seem to be used in addons, either. I'm not surprised, because that is just a travesty.
(Assignee)

Comment 9

3 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

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

Comment 10

3 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

3 years ago
Keywords: site-compat
(Assignee)

Comment 11

3 years 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

3 years ago
Enigmail 1.9 with the fix has been released.

will prepare patches.
(Assignee)

Comment 14

3 years 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

3 years 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

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

Comment 17

3 years 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

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

Comment 19

3 years 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

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

Comment 21

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

Comment 22

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

Comment 23

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

Comment 24

3 years 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

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

Comment 26

3 years 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

3 years 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
You need to log in before you can comment on or make changes to this bug.