Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

es.regexp.sticky feature verification fails #1008

Closed
chenyulun opened this issue Nov 17, 2021 · 11 comments · Fixed by #1015
Closed

es.regexp.sticky feature verification fails #1008

chenyulun opened this issue Nov 17, 2021 · 11 comments · Fixed by #1015

Comments

@chenyulun
Copy link

chenyulun commented Nov 17, 2021

es.regexp.sticky
ua: Mozilla/5.0 (Linux; U; Android 11; zh-CN; M2102K1AC Build/RKQ1.201112.002) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/57.0.2987.108 UCBrowser/12.5.5.1035 Mobile Safari/537.36

'es.regexp.sticky': function () {
    return new RegExp('a', 'y').sticky === true; // false
}

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/sticky
But the examples on the website can pass the test

{
'es.regexp.sticky': function () {
    return new RegExp('a', 'y').sticky === true; // false
}
}
```js
{
  'es.regexp.sticky': function () {
    // return new RegExp('a', 'y').sticky === true;
    var str = '#foo#';
    var regex = /foo/y;
    regex.lastIndex = 1;
    var flag1 = regex.test(str)
    regex.lastIndex = 5;
    return !regex.test(str) && flag1; // true
  }
}

by the way: In this browser,
es.regexp.constructor:

var re1 = /a/g;
RegExp(re1, 'i') == '/a/i' // false

The full core-JS patch is still not supported

23423423r34r
1223werw

@zloirock
Copy link
Owner

zloirock commented Nov 17, 2021

Sorry, I'm not sure that I understood the issue.

I have no UC Browser for testing, but in Chrome 57 both of your examples work fine.

BTW we can't use /foo/y in tests or after polyfilling since it's an extension of JS syntax and polyfills can't do it, it's a work of transpilers, however, I don't think that it's related to this issue.

Could you explain what do you mean? Is this just /tests/compat or you load / do something else?

@zloirock
Copy link
Owner

?

@chenyulun
Copy link
Author

chenyulun commented Nov 18, 2021

I wanted to test the compatibility of Core-JS across all versions of mobile browsers,so I added core-js-3.19.1.js to tests/compat/index.html,It just happened to test a UC browser in China, which encapsulates the chrome/57 kernel and does not support the verification of these two attributes. (How to test whether it is really Chrome /57),
I think these two unsupported uses, should be very few people use, and we do not have other Chrome /57 mobile devices, so ignore,
cp core-js-bundle/index.js into Project root

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Document</title>
  <script src="./core-js-bundle/index.js"></script>
</head>
<body>
  <div id="ua"></div>
  <div id="regexp_constructor"></div>
  <div id="regexp_sticky"></div>
</body>
<script>
  var ua = document.getElementById('ua');
  ua.innerHTML ='ua:' + navigator.userAgent; // + '<br/>' + 'appVersion:' + navigator.appVersion
  var sticky = document.getElementById('regexp_constructor');
  sticky.innerHTML = "es.regexp.constructor(RegExp(/a/g, 'i')):" + (RegExp(/a/g, 'i'));
  var sticky = document.getElementById('regexp_sticky');
  sticky.innerHTML = "es.regexp.sticky(new RegExp('a', 'y').sticky):" + (new RegExp('a', 'y').sticky);
</script>
</html>

chrome:
image
UC: chrome/57

wdfsdfsdf

@zloirock
Copy link
Owner

zloirock commented Nov 20, 2021

Thanks, interesting. As I wrote, now I have no environment where I can test it in UC browser and I have no ideas why it could fail. If you could dig it deeper and, if it's required, help to fix it - I would be happy.

@chenyulun
Copy link
Author

setTimeout(() => {
  console.log(navigator.userAgent)
  const re = RegExp('a', 'y')
  console.log('re.toString:', re.toString())
  console.log('re.sticky:', re.sticky)
  var str = '#foo#';
  var regex = /foo/y;
  console.log('regex.toString:', regex.toString())
  regex.lastIndex = 1;
  console.log('lastIndex:1,', regex.test(str)); // true
  regex.lastIndex = 5;
  console.log('lastIndex:5,',regex.test(str));
}, 2000);

Android UC browser download address

chrome 56 PC

3456456

UC chrome/57

uc

The toString method and the sticky attribute are not supported, but the functionality is supported

The chrome kernel version it uses is probably not real Chrome 57

@zloirock
Copy link
Owner

Interesting, thanks. We could extend UNSUPPORTED_Y logic to the .sticky flag detection. But I'm not sure is required or not to replace a native implementation that works properly, but without observable .sticky flag, to slower and not 100% complete (#810) polyfill... We can add a special case for that to the constructor, however, I'm not sure if I can do it blindly correctly enough. What do you think?

@chenyulun
Copy link
Author

I think this browser patch can be abandoned!
I have no good advice on how to fix it!
Can you judge for yourself how to deal with the problem
Finally, thank you for your support

@zloirock
Copy link
Owner

@chenyulun could you try a fix from this branch? https://github.com/zloirock/core-js/tree/uc-sticky

@chenyulun
Copy link
Author

I'll try to fix the problem, but it may take a few more days

@zloirock
Copy link
Owner

zloirock commented Nov 27, 2021

In UC Browser 12.10, I can't reproduce it - all tests passed. However, it's not 12.5.5.1035 like in your case.

image

@zloirock
Copy link
Owner

Just in case, I'll add a workaround for that from the branch above since it will not make something worse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants