Web Inspector: evaluate in console may not work when window.console is substituted Initial bug - http://code.google.com/p/chromium/issues/detail?id=70629.
Created attachment 80039 [details] Patch.
Comment on attachment 80039 [details] Patch. Can you provide a test for this?
Comment on attachment 80039 [details] Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=80039&action=review > Source/WebCore/inspector/InjectedScriptSource.js:261 > + expression = "with ((window && window.console && window.console._commandLineAPI) || {}) {\n" + expression + "\n}"; If .console is undefined then this method will fail at line 256 and not get here: inspectedWindow.console._commandLineAPI = this._commandLineAPI;
(In reply to comment #3) > (From update of attachment 80039 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80039&action=review > > > Source/WebCore/inspector/InjectedScriptSource.js:261 > > + expression = "with ((window && window.console && window.console._commandLineAPI) || {}) {\n" + expression + "\n}"; > > If .console is undefined then this method will fail at line 256 and not get here: > > inspectedWindow.console._commandLineAPI = this._commandLineAPI; Not undefined, but substituted. In any case this patch looks safe and fixes the problem.
Created attachment 80063 [details] Added test.
Comment on attachment 80063 [details] Added test. View in context: https://bugs.webkit.org/attachment.cgi?id=80063&action=review > LayoutTests/inspector/console-substituted.html:21 > + InspectorTest.evaluateInConsole("setTimeout(deleteConsole, 0)", step1); Why do you need setTimeout here? > LayoutTests/inspector/console-substituted.html:31 > + InspectorTest.evaluateInConsole("setTimeout(substituteConsole, 0)", step3); The same question for this call. > Source/WebCore/inspector/InjectedScriptSource.js:261 > + expression = "with (window || {}) {\n" + expression + "\n} "; is there a real scenario when there are no window?
(In reply to comment #6) > (From update of attachment 80063 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80063&action=review > > > LayoutTests/inspector/console-substituted.html:21 > > + InspectorTest.evaluateInConsole("setTimeout(deleteConsole, 0)", step1); > > Why do you need setTimeout here? > > > LayoutTests/inspector/console-substituted.html:31 > > + InspectorTest.evaluateInConsole("setTimeout(substituteConsole, 0)", step3); > > The same question for this call. > > > Source/WebCore/inspector/InjectedScriptSource.js:261 > > + expression = "with (window || {}) {\n" + expression + "\n} "; > > is there a real scenario when there are no window? Sure, you may be inside a closure created in function with "window" argument, which is not used by that closure. In that case "window" will be undefined (v8 will drop it).
(In reply to comment #6) > (From update of attachment 80063 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80063&action=review > > > LayoutTests/inspector/console-substituted.html:21 > > + InspectorTest.evaluateInConsole("setTimeout(deleteConsole, 0)", step1); > > Why do you need setTimeout here? Replaced with direct call. > > > LayoutTests/inspector/console-substituted.html:31 > > + InspectorTest.evaluateInConsole("setTimeout(substituteConsole, 0)", step3); > > The same question for this call. Replaced with direct call. > > > Source/WebCore/inspector/InjectedScriptSource.js:261 > > + expression = "with (window || {}) {\n" + expression + "\n} "; > > is there a real scenario when there are no window? Fixed. Since this is behind !isEvalOnCallFrame, there is probably no such scenario.
Created attachment 80176 [details] Patch.
Comment on attachment 80176 [details] Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=80176&action=review > Source/WebCore/inspector/InjectedScriptSource.js:257 > + inspectedWindow.console._commandLineAPI = this._commandLineAPI; Nit: defining non-enumerable property would be even better.
Committed r76699: <http://trac.webkit.org/changeset/76699>