Summary: | Web Inspector: RuntimeManager should not use view object WebInspector.quickConsole | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||||
Component: | Web Inspector | Assignee: | BJ Burg <bburg> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bburg, commit-queue, galpeter, joepeck, llango.u-szeged, ossy, timothy, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
BJ Burg
2014-02-02 18:52:19 PST
Created attachment 224720 [details]
patch
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. 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. Yeah, that sounds good to me. 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.
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. Created attachment 282186 [details]
Proposed Fix
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: ... Comment on attachment 282186 [details] Proposed Fix Clearing flags on attachment: 282186 Committed r202522: <http://trac.webkit.org/changeset/202522> All reviewed patches have been landed. Closing bug. |