RESOLVED FIXED126855
Web Inspector: scope chain details sidebar doesn't update values modified via console
https://bugs.webkit.org/show_bug.cgi?id=126855
Summary Web Inspector: scope chain details sidebar doesn't update values modified via...
Blaze Burg
Reported 2014-01-12 14:50:20 PST
Test case: when paused, add a new property to some object visible in the scope chain details sidebar. The new property won't be added to the tree until you take a step in the debugger.
Attachments
Patch (2.28 KB, patch)
2014-02-13 16:16 PST, Chris J. Shull
no flags
Patch (3.44 KB, patch)
2014-02-13 23:12 PST, Chris J. Shull
no flags
Patch (4.35 KB, patch)
2014-02-14 10:55 PST, Chris J. Shull
no flags
Radar WebKit Bug Importer
Comment 1 2014-01-12 14:50:38 PST
Chris J. Shull
Comment 2 2014-02-13 16:16:41 PST
Timothy Hatcher
Comment 3 2014-02-13 19:16:48 PST
Comment on attachment 224125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224125&action=review Thanks for looking into this! > Source/WebInspectorUI/UserInterface/JavaScriptLogViewController.js:228 > + WebInspector.scopeChainDetailsSidebarPanel.needsRefresh(); This would be better as an event that the sidebar observes. Having this low-level view class directly call a higher-level class is a layering violation. RuntimeManager would be a good place to fire the event that the sidebar observes.
Timothy Hatcher
Comment 4 2014-02-13 19:19:12 PST
Having RuntimeManager fire it after doing an evaluate would catch any other place we evaluate expressions.
Chris J. Shull
Comment 5 2014-02-13 23:12:35 PST
Chris J. Shull
Comment 6 2014-02-13 23:14:36 PST
Thanks for the feedback, uploaded new patch. Two things I wasn’t sure about: 1) I didn’t add any data to the event, since it seems like most events don’t include data unless something will use it. Happy to add if you want though. 2) I didn’t refactor RuntimeManager#evaluateInInspectedWindow() to not have a callback at all, since having a callback made more sense for the things which were calling that method (since they needed to have callbacks specific to the invocation, as opposed to generic like the event is used.) Plus, one of the other args changes what args get passed to the callback.
Joseph Pecoraro
Comment 7 2014-02-14 10:44:50 PST
Comment on attachment 224166 [details] Patch You need to bind.(this) on both evalCallback calls. There is one on RuntimeManager.js a few lines down (line 75 after your patches). Otherwise, just typing in the console normally (not paused) will throw an exception right?
Chris J. Shull
Comment 8 2014-02-14 10:55:28 PST
Chris J. Shull
Comment 9 2014-02-14 10:55:54 PST
Yikes! Thank you for catching that. Patch updated.
WebKit Commit Bot
Comment 10 2014-02-15 00:23:36 PST
Comment on attachment 224233 [details] Patch Clearing flags on attachment: 224233 Committed r164163: <http://trac.webkit.org/changeset/164163>
WebKit Commit Bot
Comment 11 2014-02-15 00:23:38 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.