Currently, the contextmenu items for interacting with nodes (e.g. Log Element) are only available inside the DOM tree. Since these nodes are often linked/previewed elsewhere, these contextmenu items should also be visible too.
Created attachment 312497 [details] Patch
Created attachment 312498 [details] [Image] After Patch is applied
Created attachment 312501 [details] Patch
Comment on attachment 312501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312501&action=review I really like this direction. > Source/WebInspectorUI/UserInterface/Views/ContextMenuUtilities.js:130 > + contextMenu.appendItem(WebInspector.UIString("Jump to Definition"), this._showCustomElementDefinition.bind(this)); This `this._showCustomElementDefinition` does not exist and would be an exception. You can make this an inline arrow function. > Source/WebInspectorUI/UserInterface/Views/ContextMenuUtilities.js:148 > + if (!options.excludeLogElement && !domNode.isInUserAgentShadowTree()) { Nobody uses `excludeLogElement`, lets drop it. Existing code only allows "Log Element" for Node.ELEMENT_NODE nodes but didn't for Node.TEXT_NODE and others. I think we probably want to continue that here and only do this if: domNode.nodeType() === Node.ELEMENT_NODE > Source/WebInspectorUI/UserInterface/Views/RulesStyleDetailsPanel.js:182 > newDOMFragment.appendChild(inheritedLabel); Lets move this after inheritedLabel is done getting setup or at the beginning at line 180. > Source/WebInspectorUI/UserInterface/Views/RulesStyleDetailsPanel.js:190 > + const options = { > + maxLength: 100, > + excludeRevealElement: true, > + }; > + inheritedLabel.appendChild(WebInspector.linkifyNodeReference(style.node, options)); Style: I think this would read much clearer as: label.appendChild(WebInspector.linkifyNodeReference(style.node, { maxLength: 100, excludeRevealElement: true, }); In fact any place that takes an options dictionary as the last parameter, I don't think we should have a temporary variable. I think thats something we should standardize on and break only in rare circumstances.
Comment on attachment 312501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312501&action=review >> Source/WebInspectorUI/UserInterface/Views/ContextMenuUtilities.js:130 >> + contextMenu.appendItem(WebInspector.UIString("Jump to Definition"), this._showCustomElementDefinition.bind(this)); > > This `this._showCustomElementDefinition` does not exist and would be an exception. You can make this an inline arrow function. Oops. Forgot to change this from copying. Thanks =D >> Source/WebInspectorUI/UserInterface/Views/ContextMenuUtilities.js:148 >> + if (!options.excludeLogElement && !domNode.isInUserAgentShadowTree()) { > > Nobody uses `excludeLogElement`, lets drop it. > > Existing code only allows "Log Element" for Node.ELEMENT_NODE nodes but didn't for Node.TEXT_NODE and others. I think we probably want to continue that here and only do this if: > > domNode.nodeType() === Node.ELEMENT_NODE I will be using this in <https://webkit.org/b/138941>. I think that we should add support for logging other types of nodes, seeing as they are completely valid to retrieve simply by logging the parent element and getting it via the parent's children. In these cases, however, the text should probably say "Log Node". >> Source/WebInspectorUI/UserInterface/Views/RulesStyleDetailsPanel.js:190 >> + inheritedLabel.appendChild(WebInspector.linkifyNodeReference(style.node, options)); > > Style: I think this would read much clearer as: > > label.appendChild(WebInspector.linkifyNodeReference(style.node, { > maxLength: 100, > excludeRevealElement: true, > }); > > In fact any place that takes an options dictionary as the last parameter, I don't think we should have a temporary variable. I think thats something we should standardize on and break only in rare circumstances. I've heard two different things regarding this style (the other being from Brian), and have been following the format of using const local variables. I'm happy to go either way, but I think we should decide on a rule and stick to it. Some examples: <https://webkit.org/b/168021> Web Inspector: Prefer Resources tab over Network tab when showing files <https://webkit.org/b/168709> Web Inspector: Don't show the Search tab if it's open and matches the representedObject
Created attachment 312565 [details] Patch
Comment on attachment 312565 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312565&action=review r=me > Source/WebInspectorUI/UserInterface/Views/ContextMenuUtilities.js:148 > + const options = { > + ignoreNetworkTab: true, > + ignoreSearchTab: true, > + }; > + WebInspector.showSourceCodeLocation(sourceCodeLocation, options); I'd inline this object. > Source/WebInspectorUI/UserInterface/Views/Main.css:343 > + -webkit-user-select: none; What if I want to select the selector in the sidebar or something? > Source/WebInspectorUI/UserInterface/Views/RulesStyleDetailsPanel.js:189 > + const options = { > + maxLength: 100, > + excludeRevealElement: true, > + }; > + inheritedLabel.appendChild(WebInspector.linkifyNodeReference(style.node, options)); Again, I'd inline this.
Created attachment 313327 [details] Patch
Comment on attachment 313327 [details] Patch Rejecting attachment 313327 [details] from commit-queue. New failing tests: inspector/canvas/create-canvas-contexts.html Full output: http://webkit-queues.webkit.org/results/3962111
Created attachment 313330 [details] Archive of layout-test-results from webkit-cq-03 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: webkit-cq-03 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 313337 [details] Patch
Comment on attachment 313337 [details] Patch Clearing flags on attachment: 313337 Committed r218521: <http://trac.webkit.org/changeset/218521>
All reviewed patches have been landed. Closing bug.