| Summary: | Web Inspector: re-implement breakpoints sidebar pane based on debugger presentation model. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Pavel Podivilov <podivilov> | ||||||||
| Component: | Web Inspector (Deprecated) | Assignee: | Pavel Podivilov <podivilov> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Pavel Podivilov
2011-03-05 06:46:32 PST
Created attachment 84863 [details]
Patch.
Comment on attachment 84863 [details] Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=84863&action=review > Source/WebCore/inspector/front-end/ScriptsPanel.js:1056 > + _javaScriptBreakpointsSidebarPaneBreakpointClicked: function(event) I think the code would be much cleaner if you had a Breakpoint class with corresponding methods on it. Did you consider this option? It would also result it way less changes. Created attachment 85159 [details]
Patch.
(In reply to comment #2) > (From update of attachment 84863 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=84863&action=review > > > Source/WebCore/inspector/front-end/ScriptsPanel.js:1056 > > + _javaScriptBreakpointsSidebarPaneBreakpointClicked: function(event) > > I think the code would be much cleaner if you had a Breakpoint class with corresponding methods on it. Did you consider this option? It would also result it way less changes. Moved event handlers back to JavaScriptBreakpointsSidebarPane class. I don't think Breakpoint class would make the code any cleaner, it just shadows the dependencies. Comment on attachment 85159 [details]
Patch.
Could you please explain why are you doing this refactoring? Bug title says: "re-implement breakpoints sidebar pane as a simple ui component", but I see this._model being introduced in the sidebar section class. I.e. the opposite is happening. I am fine with having model in the sidebar, but I am confused wrt what exactly is happening and why.
Created attachment 85169 [details]
Patch.
(In reply to comment #5) > (From update of attachment 85159 [details]) > Could you please explain why are you doing this refactoring? Bug title says: "re-implement breakpoints sidebar pane as a simple ui component", but I see this._model being introduced in the sidebar section class. I.e. the opposite is happening. I am fine with having model in the sidebar, but I am confused wrt what exactly is happening and why. Sorry about that, forgot to update the title. *** Bug 55822 has been marked as a duplicate of this bug. *** Committed r80705: <http://trac.webkit.org/changeset/80705> |