Summary: | Web Inspector: Links to source in CPU profiles should open Scripts (debugger) panel | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail Naganov <mnaganov> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bweinstein, commit-queue, joepeck, keishi, pfeldman, pmuellr, rik, timothy, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Mikhail Naganov
2009-12-16 06:57:32 PST
Created attachment 44973 [details]
Proposed fix
style-queue ran check-webkit-style on attachment 44973 [details] without any errors.
Comment on attachment 44973 [details] Proposed fix > WebInspector.canShowResourceForURL = function(url, preferredPanel) > WebInspector.showResourceForURL = function(url, line, preferredPanel) It would be much more clear if you made Panel's canShowResource and showResource accept URL instead of resource. That way you would be able to make canShowResourceForURL and showResourceForURL here panel-agnostic. Created attachment 45012 [details]
proposed change, comments addressed
style-queue ran check-webkit-style on attachment 45012 [details] without any errors.
Comment on attachment 45012 [details] proposed change, comments addressed > +WebInspector._choosePanelToShowResourceForURL = function(url, preferredPanel) > { > - var resource = this.resourceForURL(url); > - if (!resource) > - return false; > - > + var panel = null; > if (preferredPanel && preferredPanel in WebInspector.panels) { > - var panel = this.panels[preferredPanel]; > - if (!("showResource" in panel)) > + panel = this.panels[preferredPanel]; > + if (!("showResourceForURL" in panel)) > panel = null; > - else if ("canShowResource" in panel && !panel.canShowResource(resource)) > + else if ("canShowResourceForURL" in panel && !panel.canShowResourceForURL(url)) > panel = null; > } > + if (!panel && preferredPanel != "resources") > + return this._choosePanelToShowResourceForURL(url, "resources"); > + return panel; > +} Can we implement this a bit more simple? - Require that any preferredPanel has necessary canShow and show methods - first check against preferredPanel, in case of failure explicitly check against resources. No need to reenter method. > + > +WebInspector.showResourceForURL = function(url, line, preferredPanel) > +{ This name no longer applies. WebInspector.showScriptOrResourceForURL ? r- for confusing logic in _choosePanelToShowResourceForURL and name of the method. Created attachment 45060 [details]
Proposed fix, comments addressed
style-queue ran check-webkit-style on attachment 45060 [details] without any errors.
Comment on attachment 45060 [details] Proposed fix, comments addressed > + showSourceLineForURL: function(url, line) > + { > + this.showResource(WebInspector.resourceForURL(url)); You lose line information here. Rest is Ok. Created attachment 45064 [details]
fixed 'line' argument loss for resources panel
style-queue ran check-webkit-style on attachment 45064 [details] without any errors.
Comment on attachment 45064 [details] fixed 'line' argument loss for resources panel Clearing flags on attachment: 45064 Committed r52253: <http://trac.webkit.org/changeset/52253> All reviewed patches have been landed. Closing bug. |