752200 - regular expressions update the RegExp object of the regexp's global, not the caller's global

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Wladimir Palant]

Bug 650353 seems to have an unintended side-effect when regular expressions are used. If you get the regular expression from a JavaScript module (meaning that it lives in a different compartment now) then using this regular expression will not set RegExp.leftContext and similar properties - at least not in the compartment that is using the regular expression. So code doing something like this will fail:

    let {regexp} = Cu.import("...", null);
    console.log(regexp);  // result: /b/
    if (regexp.test(/aab/))
      console.log(RegExp.leftContext);

The second console.log() call above should result in "aa" being written to console - instead it will write an empty string because RegExp.leftContext has only been set in the compartment of the regular expression.

Not sure whether this is something worth fixing (RegExp properties are deprecated) and I would be glad to avoid accessing them in Adblock Plus but I am simply unaware of good alternatives to things like RegExp.leftContext.

Comment 1

[Luke Wagner [:luke]]

It is questionable what the "right" RegExp instance to update is.  Chrome does what FF now does (updates the regexp's origin's global's RegExp), Safari does what pre-CPG FF did (updates the caller's global's RegExp).  (Can't test IE atm)

Note: to query the regexp's global's RegExp, you can always use:

  Object.getPrototypeOf(regexp).constructor.leftContext

Let's keep this bug open, though, to track the issue.  Fixing this would be rather gross, especially when it seems like we now have the correct semantics wrt globals.  The fact that Chrome has the same behavior gives me hope that not too much web content can be depending on this.

Comment 2

[Wladimir Palant]

I added addon-compat keyword which will hopefully make sure that this issue is documented - in case this is decided to be the correct behavior. I think that in Adblock Plus I will replace regexp.test() by regexp.exec() and work with the return value only, that should take care of any issues.

Comment 3

[Wladimir Palant]

Attachment: Testcase

This effect can be seen on a regular webpage as well. This testcase takes a regular expression from a frame and runs it in the top-level document. It will then check which RegExp object got updated. My test results:

Firefox 12: both updated ("RegExp object of caller updated, RegExp object of regexp updated")
Firefox 15: only the frame updated ("RegExp object of caller not updated
RegExp object of regexp updated")
Chrome 19: same as Firefox 15
MSIE 9.0: same as Firefox 15

So I guess that this bug is going to be WONTFIX'ed.

Comment 4

[Jeff Walden [:Waldo]]

I think the post-CPG, change-the-corresponding-global's-RegExp semantics are the right ones.

I'd be happy to have a discussion about how to address your use for RegExp.leftContext at some point, to figure out some standardized way to do the same thing.  I'm not sure that this is the bug to do it, just throwing it out there as something we should do at some point.

Comment 5

[Luke Wagner [:luke]]

Waldo: comment 1 had one:
  Object.getPrototypeOf(regexp).constructor.leftContext
of course that doesn't work on Safari or old FF.  To have something that works either way, you could do:
  (new RegExp(regexp)).test(...)
and then use RegExp.leftContext like normal.

Comment 6

[Jeff Walden [:Waldo]]

To be clearer, I meant a way to avoid use of the non-standard leftContext property entirely.

Comment 7

[Danial Horton]

has this got something to do with the slowdown some users are seeing with ABP on the latest firefox nightly builds

20120502030505
http://hg.mozilla.org/mozilla-central/rev/b13bfc70bc44 is the last version to work fast

Comment 8

[Danial Horton]

can see it did now.

Comment 9

[Wladimir Palant]

(In reply to Danial Horton from comment #7)
> has this got something to do with the slowdown some users are seeing with
> ABP on the latest firefox nightly builds

It does - latest Adblock Plus development build fixes this (there is still some breakage left however, just not as important).

Comment 10

[Wladimir Palant]

(In reply to Jeff Walden [:Waldo] from comment #6)
> To be clearer, I meant a way to avoid use of the non-standard leftContext
> property entirely.

I am currently replacing code like:

if (regexp.test(foo))
{
  var a = RegExp.$1;
  var b = RegExp.leftContext;
  var c = RegExp.rightContext;
  ...
}

By:

var match = regexp.exec(foo);
if (match)
{
  var a = match[1];
  var b = match.input.substr(0, match.index);
  var c = match.input.substr(match.index + match[0].length);
  ...
}

This is cleaner (no global variables used) and doesn't exhibit issues when regular expressions are used across contexts. Unfortunately, I didn't previously know that RegExp.exec() doesn't return a plain array.

Comment 11

[Wladimir Palant]

Not blocking Adblock Plus development any more, RegExp properties are no longer being used.

Comment 12

[André Bargull [:anba]]

Marking as Won't Fix because the current semantics seem to match the current TC39 proposal at <https://github.com/tc39/proposal-regexp-legacy-features>.