Bug 29107 – LayoutTests/fast/js/kde/inbuilt_function_tostring.html fails in V8

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Tools / Tests

[Victor Wang]

toString() function in JSC and V8 does not generate same results. For example, in JSC, eval.toString()) outputs "function eval(){ [native code] }", while in V8, it outputs "function eval(x) { if(!(typeof(x)==='string')) ... }.

The layout test "LayoutTests/fast/js/kde/inbuilt_function_tostring.html" uses ShouldBe function (exact string match) to check whether the test passes, so it works in JSC but not in Chromium. This test can be modified using regexp so it works in both cases.

[Sam Weinig]

The layout tests are intended to test webkit behavior.  Changing the test will weaken the coverage.  This seems like something that should change in v8.  Not being compatible with JSC seems bad for the WebKit project.

[Adam Barth]

Re: Comment #1, I've made that exact argument to the V8 folks with few results.  IMHO, webkit/v8 should act the same as stock webkit.org.

[Mads Ager]

In V8, we print the exact function code that we receive including the exact whitespace amount.  It seems that JSC changes at least the whitespace amount.  I'm not sure I understand the reason for doing that.

We have some of the basic functions (such as eval) implemented in JavaScript in V8 and we expose the code if people print those functions.  We could change that, but I don't see the point in doing so.  Showing the code is nice and I doubt that there are any compatibility issues here.  I have seen no pages that break because of this.

I think it would be nice to change the tests that involve testing the actual toString output for functions to at least allow differences in the amount of whitespace.

[Sam Weinig]

I disagree, I think the engines should be as compatible as possible. There should be no perceivable differences to users as this will only lead to compatibility issues down the line.

[Dimitri Glazkov (Google)]

One of the conditions on which WebKit agreed to host V8 bindings was maintaining compatibility with JSC bindings. I don't see much wiggle room here.

[Pam Greene (IRC:pamg)]

Without commenting on this specific issue, there's certainly wiggle room. V8 and JSC don't behave exactly the same, and almost certainly never will. What if some site is counting on a particular function being fast, or slow? That level of "compatibility" won't be met. Likewise the exact wording of error messages, which isn't specified anywhere and varies from one engine to another. Perhaps also the exact wording of |function|.toString(). Is anyone really going to be parsing that?

[Victor Wang]

(In reply to comment #6)
> Without commenting on this specific issue, there's certainly wiggle room. V8
> and JSC don't behave exactly the same, and almost certainly never will. What if
> some site is counting on a particular function being fast, or slow? That level
> of "compatibility" won't be met. Likewise the exact wording of error messages,
> which isn't specified anywhere and varies from one engine to another. Perhaps
> also the exact wording of |function|.toString(). Is anyone really going to be
> parsing that?

There are some layout tests actually check the exact error messages and therefore fails in V8.
I agree with Pam that V8 and JSC may not always have the exact wording of error messages, and I think it would be better to change the tests so it can work for both cases.

[Pam Greene (IRC:pamg)]

(In reply to comment #7)
> There are some layout tests actually check the exact error messages and
> therefore fails in V8.

In many cases, we've just created "failing" platform results for V8, as long as the actual message also appears in the output (e.g. "expected 'message A', got 'message different-A'"). It's a simple and effective workaround that only makes my skin crawl a tiny bit. :)

[Victor Wang]

(In reply to comment #8)
> (In reply to comment #7)
> > There are some layout tests actually check the exact error messages and
> > therefore fails in V8.
> 
> In many cases, we've just created "failing" platform results for V8, as long as
> the actual message also appears in the output (e.g. "expected 'message A', got
> 'message different-A'"). It's a simple and effective workaround that only makes
> my skin crawl a tiny bit. :)

OK. We can do this workaround for these tests even though I really don't like to do it in this way.

[Sam Weinig]

You are welcome work around this on your end however you like, but I think it is inappropriate to change a test in such a manner.  I still think you should fix this bug in v8.  In the past, we have had issues with sites depending on the output of function.toString() (including that of builtins).

[Dimitri Glazkov (Google)]

(In reply to comment #10)
> You are welcome work around this on your end however you like, but I think it
> is inappropriate to change a test in such a manner.  I still think you should
> fix this bug in v8.  In the past, we have had issues with sites depending on
> the output of function.toString() (including that of builtins).

I agree w/Sam here. I've seen these types of JS hacks myself. Though I find them appalling and want the Web devs who write them to bear the brunt of their failures, alas that's not the way the Web works.

Mads and I chatted offline and we are in agreement that this is a V8 issue. However, with everything currently on the plate, I don't mind to compromise and have a local rebaseline of the test, filing a V8 bug to fix these.

[Victor Wang]

(In reply to comment #11)
> (In reply to comment #10)
> > You are welcome work around this on your end however you like, but I think it
> > is inappropriate to change a test in such a manner.  I still think you should
> > fix this bug in v8.  In the past, we have had issues with sites depending on
> > the output of function.toString() (including that of builtins).
> 
> I agree w/Sam here. I've seen these types of JS hacks myself. Though I find
> them appalling and want the Web devs who write them to bear the brunt of their
> failures, alas that's not the way the Web works.
> 
> Mads and I chatted offline and we are in agreement that this is a V8 issue.
> However, with everything currently on the plate, I don't mind to compromise and
> have a local rebaseline of the test, filing a V8 bug to fix these.

OK. I will do that.

[Anders Carlsson]

V8 is gone.