Bug 54961 - Web Inspector: refactor "script or resource" mess in scripts panel
Summary: Web Inspector: refactor "script or resource" mess in scripts panel
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Podivilov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-22 08:27 PST by Pavel Podivilov
Modified: 2011-02-24 06:38 PST (History)
10 users (show)

See Also:


Attachments
Patch. (31.93 KB, patch)
2011-02-22 08:29 PST, Pavel Podivilov
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Podivilov 2011-02-22 08:27:52 PST
Web Inspector: refactor "script or resource" mess in scripts panel.

- use sourceName instead of scriptOrResource
- replace two huge functions _addScriptToFilesMenu and _showScriptOrResource that are calling each other recursively with small one-purpose non-recursive functions
Comment 1 Pavel Podivilov 2011-02-22 08:29:12 PST
Created attachment 83320 [details]
Patch.
Comment 2 Pavel Feldman 2011-02-22 12:33:02 PST
Comment on attachment 83320 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=83320&action=review

r+ with nits and request for tests.

> LayoutTests/inspector/debugger/scripts-panel.html:30
> +    InspectorTest.runDebuggerTestSuite([

InspectorTest.runTestSuite ?

> Source/WebCore/inspector/front-end/ScriptsPanel.js:335
> +        // 1) Added script url is the first item in files select

Can you add tests for these? (I.e. make sure SourceFrame is not created). There was a huge regression once in this logic.

> Source/WebCore/inspector/front-end/ScriptsPanel.js:540
> +    _showSourceFrame: function(sourceName, addToBackForwardList, saveAsLastViewedScriptFile)

saveAsLastViewedScriptFile -> userGesture? Aslo, I'd prefer couple of wrapper functions with sound names instead of these two boolean params all over the calling code.
Comment 3 Pavel Podivilov 2011-02-24 06:38:41 PST
Committed r79555: <http://trac.webkit.org/changeset/79555>