WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
71826
Web Inspector: [refactoring] remove a bunch of methods from JavaScriptSourceFrameDelegate
https://bugs.webkit.org/show_bug.cgi?id=71826
Summary
Web Inspector: [refactoring] remove a bunch of methods from JavaScriptSourceF...
Andrey Kosyakov
Reported
2011-11-08 10:03:18 PST
Those methods that may be implemented with the help of PresentationModel and UISourceCode only don't have to be in JavaScriptsSourceFrameDelegate.
Attachments
Patch
(11.22 KB, patch)
2011-11-08 10:04 PST
,
Andrey Kosyakov
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Andrey Kosyakov
Comment 1
2011-11-08 10:04:43 PST
Created
attachment 114101
[details]
Patch
Pavel Feldman
Comment 2
2011-11-08 10:14:49 PST
Comment on
attachment 114101
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=114101&action=review
> Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js:82 > + this._model.setScriptSource(this._uiSourceCode, newContent, callback);
Nit: We could encapsulate setScriptSource in the uiSourceCode for convenience.
> Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js:374 > + this._model.removeBreakpoint(this._uiSourceCode, lineNumber);
A dozen of methods that now accept uiSourceCode as the first argument suggest that they could be declared on that class.
Pavel Feldman
Comment 3
2011-11-08 10:18:25 PST
Comment on
attachment 114101
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=114101&action=review
> Source/WebCore/inspector/front-end/ScriptsPanel.js:1183 > this._scriptsPanel._toggleBreakpointsClicked();
I am fine with declaring _scriptsPanel on the JavaScriptSourceFrame. That way you'll be able to get rid of the delegate entirely. You could either make _setScriptSourceIsBeingEdited public or move the entire JavaScriptSourceFrame into ScriptsPanel class and make everything it needs from ScriptsPanel private.
Andrey Kosyakov
Comment 4
2011-11-09 07:41:03 PST
Comment on
attachment 114101
[details]
Patch Clearing flags on attachment: 114101 Committed
r99717
: <
http://trac.webkit.org/changeset/99717
>
Andrey Kosyakov
Comment 5
2011-11-09 07:41:12 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.
Top of Page
Format For Printing
XML
Clone This Bug