Bug 60391

Created attachment 92615 [details] Proposed patch

(In reply to comment #0) > The message for ReferenceError differs between JSC and V8 and having it in the expected results file requires unnecessary platform specific expected results. Does it have to differ? Can we make the messages the same?

(In reply to comment #2) > (In reply to comment #0) > > The message for ReferenceError differs between JSC and V8 and having it in the expected results file requires unnecessary platform specific expected results. > > Does it have to differ? Can we make the messages the same? I think it would be good to have the same message. The question is which message it should be? The V8 message seems to match the one you get in Firefox (aoeu is not defined), but perhaps the JSC message (Can't find variable: aoeu) is better since "is not defined" can be confused with undefined (an undefined value of a declared variable).

(In reply to comment #2) > (In reply to comment #0) > > The message for ReferenceError differs between JSC and V8 and having it in the expected results file requires unnecessary platform specific expected results. > > Does it have to differ? Can we make the messages the same? :(

> Does it have to differ? Can we make the messages the same? I would very much like to make the messages the same. Historically when I've discussed this issue with the V8 team, they haven't been very receptive. (I can dig up the bug numbers if you're interested.) Some of the messages are generated by the bindings layer, which are easier to synchronize. Unfortunately, I believe the ReferenceError is generated by the JS engine itself.

This should be fixed by now. Can you please confirm it? All the tests passed in mac using JSC last time I tried. Thanks.

(In reply to comment #6) > This should be fixed by now. Can you please confirm it? All the tests passed in mac using JSC last time I tried. > > Thanks. Still got the same problem with a build from this morning (r87680). Here's the diff: --- /tmp/layout-test-results/fast/dom/MediaStream/no-interface-object-expected.txt 2011-05-30 13:28:59.468002394 +0200 +++ /tmp/layout-test-results/fast/dom/MediaStream/no-interface-object-actual.txt 2011-05-30 13:28:59.468002394 +0200 @@ -4,13 +4,13 @@ PASS typeof NavigatorUserMedia is "undefined" -PASS NavigatorUserMedia.prototype threw exception ReferenceError: NavigatorUserMedia is not defined. +PASS NavigatorUserMedia.prototype threw exception ReferenceError: Can't find variable: NavigatorUserMedia. PASS typeof NavigatorUserMediaError is "undefined" -PASS NavigatorUserMediaError.prototype threw exception ReferenceError: NavigatorUserMediaError is not defined. +PASS NavigatorUserMediaError.prototype threw exception ReferenceError: Can't find variable: NavigatorUserMediaError. PASS typeof NavigatorUserMediaSuccessCallback is "undefined" -PASS NavigatorUserMediaSuccessCallback.prototype threw exception ReferenceError: NavigatorUserMediaSuccessCallback is not defined. +PASS NavigatorUserMediaSuccessCallback.prototype threw exception ReferenceError: Can't find variable: NavigatorUserMediaSuccessCallback. PASS typeof NavigatorUserMediaErrorCallback is "undefined" -PASS NavigatorUserMediaErrorCallback.prototype threw exception ReferenceError: NavigatorUserMediaErrorCallback is not defined. +PASS NavigatorUserMediaErrorCallback.prototype threw exception ReferenceError: Can't find variable: NavigatorUserMediaErrorCallback. PASS successfullyParsed is true TEST COMPLETE

Sorry, I assigned the bug to Tommyw accidentally. I'm resetting it.

The patch looks just fine, but just for clarification have you verified it in V8?

(In reply to comment #9) > The patch looks just fine, but just for clarification have you verified it in V8? No, but the chromium ews seems happy.

(In reply to comment #10) > (In reply to comment #9) > > The patch looks just fine, but just for clarification have you verified it in V8? > > No, but the chromium ews seems happy. Yes, it looks like. The Chromium ews bot doesn't seem to turn red when some tests fail (which was the case), but fortunately these were completely unrelated to this. So it seems it's working fine in V8 too.

Comment on attachment 92615 [details] Proposed patch Filed a new bug for the continued discussion about aligning the error messages between JSC and V8: http://webkit.org/b/62540

Comment on attachment 92615 [details] Proposed patch Clearing flags on attachment: 92615 Committed r89203: <http://trac.webkit.org/changeset/89203>

All reviewed patches have been landed. Closing bug.