Closed Bug 1219757 Opened 9 years ago Closed 8 years ago

Remove nonstandard RegExp.multiline global switch

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: claude.pache, Assigned: arai)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [DocArea=JS])

Attachments

(13 files, 3 obsolete files)

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
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.
Blocks: 1103158
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.
Attached patch Draft patch for Enigmail (obsolete) — Splinter Review
Any Enigmail developer available in bmo?
Attached patch Draft patch for Enigmail (obsolete) — Splinter Review
Just noticed they have git repo.
Will post there too.
Attachment #8681068 - Attachment is obsolete: true
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.
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
Depends on: 1220457
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
Attached file WIP patches (obsolete) —
Here's draft patches to remove RegExp.multiline and related things.
Patch in bug 1220457 should be backed out before applying them.
Keywords: site-compat
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
Enigmail 1.9 with the fix has been released.

will prepare patches.
Just backs out bug 1220457 patch.
Assignee: nobody → arai.unmht
Attachment #8691925 - Attachment is obsolete: true
Attachment #8729119 - Flags: review?(till)
Removed RegExp.multiline and RegExp.$* accessors, and tests for them.
Attachment #8729122 - Flags: review?(till)
Now RegExpObject::create and RegExpObject::createNoStatics are same, so merged to RegExpObject::create.
Attachment #8729130 - 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+
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.