Created attachment 103184 [details]
Load and follow instructions to see JavaScript console bug

This attachment didn't take the first time, so adding it now.

Created attachment 103185 [details]
What I see when I follow the instructions on previous attachment

This screenshot seems to demonstrate that, when the JavaScript console is in a strict context and tries to evaluate an expression, it first surrounds the expression with a "with" statement and then tries to evaluate the resulting string in a strict context. This of course cannot work, since "with" is illegal in strict mode.

Created attachment 103186 [details]
Demonstrates that the bug also occurs on the released Safari 5.0.6

Confirmed that this bug is still occuring in the latest version of Safari (Version 5.1.3 (7534.53.10)) and in the latest Webkit Nightly build (r108593).

Created attachment 128509 [details]
the latest error message and traceback from the attached test case (Webkit r108593)

This issue seems to be duplicated in: https://bugs.webkit.org/show_bug.cgi?id=83267 and still occurs in Safari 6.0.2 (8536.26.17)

*** Bug 83267 has been marked as a duplicate of this bug. ***

Created attachment 194361 [details]
Proposed Change

Comment on attachment 194361 [details]
Proposed Change

View in context: https://bugs.webkit.org/attachment.cgi?id=194361&action=review

r=me

> Source/WebCore/ChangeLog:14
> +        console to stay in strict mode. This also allows users to opt-into strict mode by prefixing

s/stay in/stay out of/ -- we're evaluating inside a strict mode function, but then injecting a non-strict function.

Comment on attachment 194361 [details]
Proposed Change

View in context: https://bugs.webkit.org/attachment.cgi?id=194361&action=review

> Source/WebCore/inspector/InjectedScriptSource.js:584
>              var result = evalFunction.call(object, expression);

I think we should surround this case in an empty function expression. That way, the behavior of things like "var" won't change.

Comment on attachment 194361 [details]
Proposed Change

Attachment 194361 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17290116

New failing tests:
inspector/debugger/debugger-pause-in-eval-script.html
inspector/debugger/mutation-observer-suspend-while-paused.html
inspector/console/console-eval-syntax-error.html
inspector/console/console-eval-fake.html
svg/custom/marker-orient-auto.html
inspector/extensions/extensions-sidebar.html
inspector/extensions/extensions-eval.html
inspector/console/command-line-api.html

Created attachment 194368 [details]
Archive of layout-test-results from webkit-ews-11

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: <class 'webkitpy.common.config.ports.MacWK2Port'>  Platform: Mac OS X 10.8.2

Comment on attachment 194361 [details]
Proposed Change

Attachment 194361 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17207558

New failing tests:
inspector/debugger/debugger-pause-in-eval-script.html
http/tests/inspector/console-cd-completions.html
inspector/debugger/mutation-observer-suspend-while-paused.html
inspector/console/console-eval-syntax-error.html
inspector/console/console-eval-fake.html
inspector/extensions/extensions-sidebar.html
inspector/extensions/extensions-eval.html
inspector/console/command-line-api.html

Created attachment 194373 [details]
Archive of layout-test-results from gce-cr-linux-06

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

Comment on attachment 194361 [details]
Proposed Change

Attachment 194361 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17112655

New failing tests:
inspector/debugger/debugger-pause-in-eval-script.html
inspector/debugger/mutation-observer-suspend-while-paused.html
inspector/console/console-eval-syntax-error.html
inspector/console/console-eval-fake.html
inspector/extensions/extensions-sidebar.html
inspector/extensions/extensions-eval.html
inspector/console/command-line-api.html

Created attachment 194454 [details]
Archive of layout-test-results from webkit-ews-04

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: <class 'webkitpy.common.config.ports.MacPort'>  Platform: Mac OS X 10.8.2

I have a better patch coming soon.

Created attachment 194718 [details]
Proposed Change (Take 2)

Comment on attachment 194718 [details]
Proposed Change (Take 2)

Attachment 194718 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17235424

New failing tests:
inspector/console/command-line-api.html

Created attachment 194725 [details]
Archive of layout-test-results from gce-cr-linux-02 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-02  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

Created attachment 194726 [details]
Proposed Change (Take 3)

Comment on attachment 194726 [details]
Proposed Change (Take 3)

Attachment 194726 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17311055

New failing tests:
http/tests/inspector/console-resource-errors.html

Created attachment 194733 [details]
Archive of layout-test-results from webkit-ews-05 for mac-future

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-future  Platform: Mac OS X 10.8.2

Comment on attachment 194726 [details]
Proposed Change (Take 3)

Attachment 194726 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17304056

New failing tests:
http/tests/inspector/console-resource-errors.html

Created attachment 194734 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2

Created attachment 194752 [details]
Proposed Change (Take 4)

Comment on attachment 194752 [details]
Proposed Change (Take 4)

View in context: https://bugs.webkit.org/attachment.cgi?id=194752&action=review

> Source/WebCore/inspector/InjectedScriptHost.h:109
> +    bool evaluateReturnsEvalFunction() { return true; }

So far we were hiding all the differences like this behind Script* boundary. Also, could we instead introduce evaluateFunction getter that v8 will fail to return instead of this boolean flag? That way InjectedScriptSource will be compiler friendly.

(In reply to comment #27)
> (From update of attachment 194752 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=194752&action=review
> 
> > Source/WebCore/inspector/InjectedScriptHost.h:109
> > +    bool evaluateReturnsEvalFunction() { return true; }
> 
> So far we were hiding all the differences like this behind Script* boundary. Also, could we instead introduce evaluateFunction getter that v8 will fail to return instead of this boolean flag? That way InjectedScriptSource will be compiler friendly.

How is this not patch not compiler friendly?

I considered exposing evaluateFunction. But I didn't want both evaluate and evaluateFunction in the JSC path. We want to have only one way to evaluate.

Comment on attachment 194752 [details]
Proposed Change (Take 4)

Clearing flags on attachment: 194752

Committed r146840: <http://trac.webkit.org/changeset/146840>

All reviewed patches have been landed.  Closing bug.

Created attachment 195019 [details]
The bug still happens on the latest Nightly (part 1)

I see that this bug is closed. But the attached screenshot shows that the latest Nightly still suffers from this bug.

Created attachment 195020 [details]
The bug still happens on the latest Nightly (part 2)

WebKit r146818 was built on 25 March 2013 and is a 47.5 MB download.

This landed in r146840. Next build will have it.

WebKit r146855 was built on 26 March 2013 and is a 47.4 MB download.

Give it a try now.

Created attachment 195138 [details]
Screenshot showing that the bug isn't fixed -- Part 1

(In reply to comment #34)
> WebKit r146855 was built on 26 March 2013 and is a 47.4 MB download.
> 
> Give it a try now.

I just updated to the latest Nightly, which self-identifies as r146901  and  6.0.3 (8536.28.10, 537+). Attached is the first screenshot demonstrating that the bug still isn't fixed, but manifests differently. Second screenshot in next attachment

Created attachment 195139 [details]
Screenshot showing that the bug isn't fixed -- Part 2

Thanks Mark. Apparently my strict mode test stopped triggering strict mode due to a bug — and I totally missed this.

I have a fix coming right up.

Reopening.

Created attachment 195156 [details]
Proposed Additional Change

Comment on attachment 195156 [details]
Proposed Additional Change

r=me

Comment on attachment 195156 [details]
Proposed Additional Change

https://trac.webkit.org/r146937

Migrating to new component. The patch has undoubtedly bitrotted.

This has been fixed in TOT and Safari 7 with a different approach.