RESOLVED FIXED 70885
Web Inspector: [refactoring] get JS-specific methods out of SourceFrame 
https://bugs.webkit.org/show_bug.cgi?id=70885
Summary Web Inspector: [refactoring] get JS-specific methods out of SourceFrame
Andrey Kosyakov
Reported 2011-10-26 01:33:54 PDT
Methods that are JS/debugger specific belong to JavaScriptSourceFrame. This also lets us get rid of SourceFrameDelegate.
Attachments
patch (45.80 KB, patch)
2011-10-26 01:57 PDT, Andrey Kosyakov 		no flags
DetailsFormatted DiffDiff
patch (46.54 KB, patch)
2011-10-27 06:13 PDT, Andrey Kosyakov 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
patch (fixed test) (47.28 KB, patch)
2011-10-27 10:15 PDT, Andrey Kosyakov 		pfeldman: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Andrey Kosyakov
Comment 1 2011-10-26 01:57:38 PDT
Created attachment 112471 [details] patch
Pavel Podivilov
Comment 2 2011-10-27 01:21:36 PDT
Comment on attachment 112471 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=112471&action=review Looks good. > Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js:92 > + setReadonly: function(readOnly) Please use camelCase: setReadOnly.
Pavel Podivilov
Comment 3 2011-10-27 01:24:51 PDT
(In reply to comment #2) > (From update of attachment 112471 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=112471&action=review > > Looks good. > > > Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js:92 > > + setReadonly: function(readOnly) > > Please use camelCase: setReadOnly. BTW, how does that work? In base class it is declared as "setReadOnly".
Andrey Kosyakov
Comment 4 2011-10-27 06:13:16 PDT
Created attachment 112671 [details] patch
Andrey Kosyakov
Comment 5 2011-10-27 06:14:04 PDT
(In reply to comment #3) > > BTW, how does that work? In base class it is declared as "setReadOnly". It doesn't, though the effect was somewhat subtle, so I missed it while testing. Thanks for spotting this!
WebKit Review Bot
Comment 6 2011-10-27 09:45:53 PDT
Comment on attachment 112671 [details] patch Attachment 112671 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10230461 New failing tests: inspector/debugger/source-frame.html
Andrey Kosyakov
Comment 7 2011-10-27 10:15:56 PDT
Created attachment 112700 [details] patch (fixed test)
Yury Semikhatsky
Comment 8 2011-10-28 07:04:46 PDT
Comment on attachment 112700 [details] patch (fixed test) View in context: https://bugs.webkit.org/attachment.cgi?id=112700&action=review > Source/WebCore/ChangeLog:82 > + Web Inspector: JS exception in JavaScriptSourceFrame.onShowPopover/showObjectPopover() Remove this. > Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js:60 > + if (this._popoverHelper) this._popoverHelper seems to always exist, no need to check?
Andrey Kosyakov
Comment 9 2011-11-07 07:10:20 PST
Manually committed r99410: http://trac.webkit.org/changeset/99410 with yurys' comments addressed.
Note You need to log in before you can comment on or make changes to this bug.