Bug 128092 – Web Inspector: RuntimeManager should not use view object WebInspector.quickConsole

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Blaze Burg]

This seems to be a straightforward model/view layering violation. The "active" context identifier should live in the RuntimeManager instead.

With this fixed, we will be able to use RuntimeAgent.evaluateInInspectedWindow() within inspector-protocol tests that test the model, instead of blitzing raw "RuntimeAgent.evaluate" messages.

[Radar WebKit Bug Importer]

<rdar://problem/15966526>

[Gergő Balogh]

Created attachment 224720 [details]
patch

[Timothy Hatcher]

Comment on attachment 224720 [details]
patch

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

> Source/WebInspectorUI/UserInterface/RuntimeManager.js:76
> +        if (!contextId) {
> +            contextId = WebInspector.quickConsole.executionContextIdentifier;
> +        }

Passing in the contextId (preferably contextIdentifier) is the right approach.

The idea of this bug was to remove the use of WebInspector.quickConsole altogether, not just hide it behind an optional argument.

I also don't see where anyone is passing a context to evaluateInInspectedWindow in this patch, so nothing has really changed.

[Blaze Burg]

I think the correct fix here is to store/load the last user-selected execution context on the WebInspector.Frame. The frame already stores WebInspector.ExecutionContext's and WebInspector.ExecutionContextList.

[Timothy Hatcher]

Yeah, that sounds good to me.

[Gergő Balogh]

Created attachment 227844 [details]
proposed patch

I can not find any case when the value of contextID in the original version is not undefined. Maybe I miss something, but I think this is a left over from other fixes. Based on these guess I simple remove it.

[Timothy Hatcher]

comtextId is pickable from a popup menu in the console input area when there are Safari extensions or frames on the page. Removing this line breaks that feature.

[Blaze Burg]

Created attachment 282186 [details]
Proposed Fix

[Joseph Pecoraro]

Comment on attachment 282186 [details]
Proposed Fix

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

> LayoutTests/inspector/runtime/change-execution-context-identifier-expected.txt:16
> +Passphrase in selected frame: rosewater
> +PASS: The passphrase should match the phrase defined in the subframe.

Style: InspectorTest.expectThat would output PASS: ...

[WebKit Commit Bot]

Comment on attachment 282186 [details]
Proposed Fix

Clearing flags on attachment: 282186

Committed r202522: <http://trac.webkit.org/changeset/202522>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.