New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
HW: ----
NextAction: ----
OS: ----
Priority: ----
Type: ----

Blocking:
issue chromium:571363



Sign in to add a comment

(/a/.__proto__ == "//") throws an exception

Reported by a.goed...@googlemail.com, Dec 21 2015

Issue description

Version: 4.8.271.6
OS: linux
Architecture: amd64

What steps will reproduce the problem?
1. run (/a/.__proto__ == "//")

What is the expected output?

This used to evaluate to false.

What do you see instead?

This throws an exception, when /a/.__proto__.toString is being called during the comparison. More specifically, in my chromium 48:

Uncaught TypeError: Method RegExp.prototype.toString called on incompatible receiver [object Object]
    at RegExp.toString (native)
    at <anonymous>:2:15
    at Object.InjectedScript._evaluateOn (<anonymous>:875:140)
    at Object.InjectedScript._evaluateAndWrap (<anonymous>:808:34)
    at Object.InjectedScript.evaluate (<anonymous>:664:21)


This code used to be a common method to detect safari browsers. As such it is still present in some websites/applications. I am not sure if this is really a bug, however it breaks existing code.

I have filed a corresponding bug at chromium there: 

https://code.google.com/p/chromium/issues/detail?id=571363
 
this is working as expected, as RegExp.prototype is a plain object, and its `toString()` method is non-generic due to its use of `this.source`, and `this.flags` which in turn depends on various other RegExp prototype accessors, all of which are non-generic by default.

http://tc39.github.io/ecma262/#sec-regexp.prototype.tostring
http://tc39.github.io/ecma262/#sec-get-regexp.prototype.source
http://tc39.github.io/ecma262/#sec-get-regexp.prototype.flags
http://tc39.github.io/ecma262/#sec-get-regexp.prototype.multiline
etc

There has been some discussion on making these values behave differently when the receiver is a non-regexp, but it hasn't been decided yet.

How much code on the web is running into this exception?
To be honest, I have no idea how many people actually use this. However, simply searching for browser detection you will regularly come across this exact piece of code, so I assume quite a few people did use it.
Cc: littledan@chromium.org yangguo@chromium.org
Status: Available (was: Untriaged)
Let's proceed with caution in making this change. With a patch at https://codereview.chromium.org/1543723002 , I propose to revert to the old behavior and install a UseCounter for how often this comes up. Depending on how well evangelism to websites goes, and how often it turns out this comes up, we'll be able to decide whether we can make this change with the UseCounter.
Project Member

Comment 4 by 76821325...@developer.gserviceaccount.com, Dec 22 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/98f819c3e0c92d54a306cdacadda73cf96d21b52

commit 98f819c3e0c92d54a306cdacadda73cf96d21b52
Author: littledan <littledan@chromium.org>
Date: Tue Dec 22 06:35:22 2015

Add web compat workarounds for ES2015 RegExp semantics

Unexpectedly, websites depend on doing feature testing with
RegExp.prototype.sticky and browser testing with RegExp.prototype.toString().
ES2015 newly throws exceptions for both of these. In order to enable shipping
new ES2015 semantics, this patch puts in narrow workarounds for those two
cases, keeping their old behavior. UseCounters are added for how often
those particular cases come up, so we can see if it can be deprecated.

R=yangguo
BUG= v8:4637 , v8:4617 
LOG=Y
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1543723002

Cr-Commit-Position: refs/heads/master@{#32997}

[modify] http://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52/include/v8.h
[modify] http://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52/src/js/harmony-regexp.js
[modify] http://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52/src/js/regexp.js
[modify] http://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52/src/runtime/runtime-internal.cc
[modify] http://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52/src/runtime/runtime.h
[modify] http://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52/test/cctest/test-regexp.cc
[modify] http://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52/test/mjsunit/es6/regexp-flags.js
[modify] http://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52/test/webkit/fast/js/kde/RegExp-expected.txt
[modify] http://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52/test/webkit/fast/regex/toString-expected.txt

Project Member

Comment 5 by 76821325...@developer.gserviceaccount.com, Dec 22 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/08a1d1a28847caf62a996d93f11da982d2649061

commit 08a1d1a28847caf62a996d93f11da982d2649061
Author: bmeurer <bmeurer@chromium.org>
Date: Tue Dec 22 07:37:09 2015

Revert of Add web compat workarounds for ES2015 RegExp semantics (patchset #3 id:40001 of https://codereview.chromium.org/1543723002/ )

Reason for revert:
Breaks nosnap: http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20nosnap/builds/5883

Original issue's description:
> Add web compat workarounds for ES2015 RegExp semantics
>
> Unexpectedly, websites depend on doing feature testing with
> RegExp.prototype.sticky and browser testing with RegExp.prototype.toString().
> ES2015 newly throws exceptions for both of these. In order to enable shipping
> new ES2015 semantics, this patch puts in narrow workarounds for those two
> cases, keeping their old behavior. UseCounters are added for how often
> those particular cases come up, so we can see if it can be deprecated.
>
> R=yangguo
> BUG= v8:4637 , v8:4617 
> LOG=Y
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel
>
> Committed: https://crrev.com/98f819c3e0c92d54a306cdacadda73cf96d21b52
> Cr-Commit-Position: refs/heads/master@{#32997}

TBR=yangguo@google.com,yangguo@chromium.org,littledan@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= v8:4637 , v8:4617 

Review URL: https://codereview.chromium.org/1546493003

Cr-Commit-Position: refs/heads/master@{#32999}

[modify] http://crrev.com/08a1d1a28847caf62a996d93f11da982d2649061/include/v8.h
[modify] http://crrev.com/08a1d1a28847caf62a996d93f11da982d2649061/src/js/harmony-regexp.js
[modify] http://crrev.com/08a1d1a28847caf62a996d93f11da982d2649061/src/js/regexp.js
[modify] http://crrev.com/08a1d1a28847caf62a996d93f11da982d2649061/src/runtime/runtime-internal.cc
[modify] http://crrev.com/08a1d1a28847caf62a996d93f11da982d2649061/src/runtime/runtime.h
[modify] http://crrev.com/08a1d1a28847caf62a996d93f11da982d2649061/test/cctest/test-regexp.cc
[modify] http://crrev.com/08a1d1a28847caf62a996d93f11da982d2649061/test/mjsunit/es6/regexp-flags.js
[modify] http://crrev.com/08a1d1a28847caf62a996d93f11da982d2649061/test/webkit/fast/js/kde/RegExp-expected.txt
[modify] http://crrev.com/08a1d1a28847caf62a996d93f11da982d2649061/test/webkit/fast/regex/toString-expected.txt

Project Member

Comment 6 by 76821325...@developer.gserviceaccount.com, Dec 22 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/424ef009a5d394ca212de8b8c91b963e424f7e72

commit 424ef009a5d394ca212de8b8c91b963e424f7e72
Author: littledan <littledan@chromium.org>
Date: Tue Dec 22 09:16:30 2015

Reland of Add web compat workarounds for ES2015 RegExp semantics (patchset #3 id:40001 of https://codereview.chromium.org/1543723002/ )

Unexpectedly, websites depend on doing feature testing with
RegExp.prototype.sticky and browser testing with RegExp.prototype.toString().
ES2015 newly throws exceptions for both of these. In order to enable shipping
new ES2015 semantics, this patch puts in narrow workarounds for those two
cases, keeping their old behavior. UseCounters are added for how often
those particular cases come up, so we can see if it can be deprecated.

This reland replaces problematic legacy const usage with var, to
avoid issues with nosnap builds.

R=yangguo
CC=bmeurer
BUG= v8:4637 , v8:4617 
LOG=Y
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1545633002

Cr-Commit-Position: refs/heads/master@{#33002}

[modify] http://crrev.com/424ef009a5d394ca212de8b8c91b963e424f7e72/include/v8.h
[modify] http://crrev.com/424ef009a5d394ca212de8b8c91b963e424f7e72/src/js/harmony-regexp.js
[modify] http://crrev.com/424ef009a5d394ca212de8b8c91b963e424f7e72/src/js/regexp.js
[modify] http://crrev.com/424ef009a5d394ca212de8b8c91b963e424f7e72/src/runtime/runtime-internal.cc
[modify] http://crrev.com/424ef009a5d394ca212de8b8c91b963e424f7e72/src/runtime/runtime.h
[modify] http://crrev.com/424ef009a5d394ca212de8b8c91b963e424f7e72/test/cctest/test-regexp.cc
[modify] http://crrev.com/424ef009a5d394ca212de8b8c91b963e424f7e72/test/mjsunit/es6/regexp-flags.js
[modify] http://crrev.com/424ef009a5d394ca212de8b8c91b963e424f7e72/test/webkit/fast/js/kde/RegExp-expected.txt
[modify] http://crrev.com/424ef009a5d394ca212de8b8c91b963e424f7e72/test/webkit/fast/regex/toString-expected.txt

OK, the old behavior should be restored for now.
Thanks for the quick response!
Thanks for the bug report! Keep them coming.
Project Member

Comment 10 by 76821325...@developer.gserviceaccount.com, Dec 26 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3cdcef55d54c954da0eb448301ba53ee23ea37f4

commit 3cdcef55d54c954da0eb448301ba53ee23ea37f4
Author: littledan <littledan@chromium.org>
Date: Sat Dec 26 04:19:20 2015

Thread up two new V8 UseCounters for ES2015 RegExp compat

These UseCounters were added in the V8 patch at
https://codereview.chromium.org/1545633002
They track how often web compatibility-driven workarounds to the
ES2015 spec are triggered. If the frequency goes down over time,
then we can remove one or both of them.

R=japhet@chromium.org
CC=yangguo
BUG= v8:4637 , v8:4617 

Review URL: https://codereview.chromium.org/1547143002

Cr-Commit-Position: refs/heads/master@{#366891}

[modify] http://crrev.com/3cdcef55d54c954da0eb448301ba53ee23ea37f4/third_party/WebKit/Source/bindings/core/v8/V8PerIsolateData.cpp
[modify] http://crrev.com/3cdcef55d54c954da0eb448301ba53ee23ea37f4/third_party/WebKit/Source/core/frame/UseCounter.h

Cc: -littledan@chromium.org
Owner: littledan@chromium.org
Status: Fixed (was: Available)
This was fixed a while ago, with changes to flags and source in our implementation as well as the standard to allow them to be called on RegExp.prototype. The expression now evaluates to false.

Sign in to add a comment