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

Enable @@toStringTag by default #1383

Merged
merged 4 commits into from
Aug 5, 2016
Merged

Conversation

tcare
Copy link
Contributor

@tcare tcare commented Aug 3, 2016

  • Enabled @@toStringTag by default. Updated baselines.
  • Removed the legacy helpers. We only have one special case now; any external object that would have resulted in [object Object] under new rules reverts back to the class name. This retains compat with both DOM and WinRT.
  • Fixed the coverage for proxy which was outdated.
  • Fixed OS 8331493, a nightly test that was failing.

@tcare
Copy link
Contributor Author

tcare commented Aug 3, 2016

Also fixes OS 8331493, a nightly test that was failing.

@akroshg @Yongqu please review

@@ -384,8 +384,7 @@ namespace Js
Var tag = JavascriptOperators::GetProperty(thisArgAsObject, PropertyIds::_symbolToStringTag, scriptContext); // Let tag be the result of Get(O, @@toStringTag).

// 17. Return the String that is the result of concatenating "[object ", tag, and "]".
if (tag != nullptr && JavascriptString::Is(tag))
{
auto buildToString = [&scriptContext](Var tag) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious what's the expected output of Object.getOwnPropertyNames(Math)? should we have Symbol.toString output as well?

@dilijev
Copy link
Contributor

dilijev commented Aug 3, 2016

Also fixes OS 8331493, a nightly test that was failing.

nit: add this text to the PR description so it will be automatically included in the merge commit description.

@akroshg
Copy link
Contributor

akroshg commented Aug 3, 2016

Looks good to me - it would be be good to remove -tostringtag flag from the rlexe.xml (makes the xml cleaner and when we get rid of this flag completely it will be easy transition). #Resolved

@tcare
Copy link
Contributor Author

tcare commented Aug 3, 2016

Sounds good.

@Yongqu
Copy link
Contributor

Yongqu commented Aug 5, 2016

:shipit:

@chakrabot chakrabot merged commit 8061516 into chakra-core:master Aug 5, 2016
chakrabot pushed a commit that referenced this pull request Aug 5, 2016
Merge pull request #1383 from tcare:tostringtag

- Enabled @@toStringTag by default. Updated baselines.
- Removed the legacy helpers. We only have one special case now; any external object that would have resulted in [object Object] under new rules reverts back to the class name. This retains compat with both DOM and WinRT.
- Fixed the coverage for proxy which was outdated.
- Fixed OS 8331493, a nightly test that was failing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants