Summary: | Web Inspector: factor out node search controller from inspector.js | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andrey Kosyakov <caseq> | ||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Andrey Kosyakov <caseq> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | apavlov, dglazkov, keishi, loislo, peter+ews, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit-ews, webkit.review.bot, yurys | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Attachments: |
|
Description
Andrey Kosyakov
2013-03-19 06:24:13 PDT
Created attachment 193810 [details]
Patch
Comment on attachment 193810 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193810&action=review > Source/WebCore/inspector/front-end/AdvancedSearchController.js:47 > + return WebInspector.KeyboardShortcut.makeDescriptor("f", WebInspector.KeyboardShortcut.Modifiers.CtrlOrMeta | WebInspector.KeyboardShortcut.Modifiers.Shift); It was Alt > Source/WebCore/inspector/front-end/InspectorFrontendAPI.js:110 > + if (WebInspector.nodeSearchController) This is now called inspectElementMode Created attachment 193854 [details]
Patch
(In reply to comment #2) > (From update of attachment 193810 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=193810&action=review > > > Source/WebCore/inspector/front-end/AdvancedSearchController.js:47 > > + return WebInspector.KeyboardShortcut.makeDescriptor("f", WebInspector.KeyboardShortcut.Modifiers.CtrlOrMeta | WebInspector.KeyboardShortcut.Modifiers.Shift); > > It was Alt Oops! Thanks, rolled this part out. > > Source/WebCore/inspector/front-end/InspectorFrontendAPI.js:110 > > + if (WebInspector.nodeSearchController) > > This is now called inspectElementMode Renamed to elementInspectionController. Comment on attachment 193854 [details] Patch Attachment 193854 [details] did not pass cr-linux-debug-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17192608 Comment on attachment 193854 [details] Patch Attachment 193854 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17213648 Comment on attachment 193854 [details] Patch Attachment 193854 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17242092 Comment on attachment 193854 [details] Patch Attachment 193854 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17181606 Comment on attachment 193854 [details] Patch Attachment 193854 [details] did not pass cr-android-ews (chromium-android): Output: http://webkit-commit-queue.appspot.com/results/17196514 Created attachment 193860 [details]
Patch
Comment on attachment 193860 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193860&action=review > Source/WebCore/WebCore.vcproj/WebCore.vcproj:77701 > + RelativePath="..\inspector\front-end\ElementInspectionController.js" InspectElementModeController? > Source/WebCore/inspector/front-end/ElementInspectionController.js:34 > + this.toggleSearchButton = new WebInspector.StatusBarButton(WebInspector.UIString("Select an element in the page to inspect it."), "node-search-status-bar-item"); Lets hide it. > Source/WebCore/inspector/front-end/inspector.js:97 > + mainStatusBar.insertBefore(this.elementInspectionController.toggleSearchButton.element, bottomStatusBarContainer); You should encapsulate (hide) toggleSearchButton. > Source/WebCore/inspector/front-end/inspector.js:677 > + section.addKey(elementInspectionShortcut, WebInspector.UIString("Select node to inspect")); Why is "Select node to inspect" string here? > Source/WebCore/inspector/front-end/inspector.js:969 > + if (WebInspector.elementInspectionController && WebInspector.elementInspectionController.enabled()) { Can it be missing? Created attachment 194038 [details]
Patch
(In reply to comment #11) > View in context: https://bugs.webkit.org/attachment.cgi?id=193860&action=review > > > Source/WebCore/WebCore.vcproj/WebCore.vcproj:77701 > > + RelativePath="..\inspector\front-end\ElementInspectionController.js" > > InspectElementModeController? Done. > > Source/WebCore/inspector/front-end/ElementInspectionController.js:34 > > + this.toggleSearchButton = new WebInspector.StatusBarButton(WebInspector.UIString("Select an element in the page to inspect it."), "node-search-status-bar-item"); > > Lets hide it. Done. > > Source/WebCore/inspector/front-end/inspector.js:97 > > + mainStatusBar.insertBefore(this.elementInspectionController.toggleSearchButton.element, bottomStatusBarContainer); > > You should encapsulate (hide) toggleSearchButton. Done. > > Source/WebCore/inspector/front-end/inspector.js:677 > > + section.addKey(elementInspectionShortcut, WebInspector.UIString("Select node to inspect")); > > Why is "Select node to inspect" string here? This is done consistently with other shortcuts there. Am I missing something? > > Source/WebCore/inspector/front-end/inspector.js:969 > > + if (WebInspector.elementInspectionController && WebInspector.elementInspectionController.enabled()) { > > Can it be missing? Yes, worker inspectors don't have it. Comment on attachment 194038 [details]
Patch
Wrong patch?
Created attachment 194593 [details]
Patch
(In reply to comment #14) > (From update of attachment 194038 [details]) > Wrong patch? Yup, sorry for the mess. Fixed! Committed r147105: <http://trac.webkit.org/changeset/147105> |