Like Firebug, the Web inspector needs an image preview. Right now, you have to click on the url to see the image and go back to the Elements panel. This is a bad workflow when you're trying to find which rule sets a background image for example.
Created attachment 67387 [details] [IMAGE] Screenshot of image preview popovers in the DOM tree and Styles sidebar I have a patch locally that implements the feature, but in the Styles sidebar a "propertyName: propertyValue" tooltip gets in the way. This is a call for discussion of a good way to mitigate this problem.
Looks neat. Some visual comments: * We should put a 1px rgb 128,128,128 border around the image. * The popover arow in the first shot should be over the first character of the link text. It looks like it si just floating in space right now.
Another thing: we should disable the tooltip and put the image URL in this popover. The second shot shows both the popover and a tooltip which looks bad.
(In reply to comment #2) > Looks neat. > > Some visual comments: > > * We should put a 1px rgb 128,128,128 border around the image. Will do. > * The popover arow in the first shot should be over the first character of the link text. It looks like it si just floating in space right now. The popover is activated for the entire treeoutline item (highlighted in light blue). Perhaps I chose too wide an area for the popup activation (well, not without reason: the popup displays the IMG-specified image dimensions and renders the image with the correct repeat/repeat-x/repeat-y value, if applicable, which makes no sense for "pure" images). Do you think only the link should be the popup activator rather than the entire IMG element?
(In reply to comment #3) > Another thing: we should disable the tooltip and put the image URL in this popover. The second shot shows both the popover and a tooltip which looks bad. This also occurred to me while I was thinking about the data presentation, but this approach will make the Styles UI inconsistent: every "property container" (a LI containing SPANs for the property name and value) has a "title" with the full property signature. This is what you are seeing while the mouse is hovering over the URL (since the mouse events bubble up to the LI). Thus, some properties will have this tooltip and others won't. Do you think this is acceptable?
Created attachment 126772 [details] Patch
Comment on attachment 126772 [details] Patch Attachment 126772 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11507854 New failing tests: inspector/elements/elements-img-tooltip.html
Comment on attachment 126772 [details] Patch r- per failing test.
Created attachment 126930 [details] Patch
Comment on attachment 126930 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126930&action=review > Source/WebCore/inspector/front-end/ElementsTreeOutline.js:721 > + this._dimensions = { offsetWidth: properties[0], offsetHeight: properties[1], naturalWidth: properties[2], naturalHeight: properties[3] }; You should place all the functionality into the same place, there is no need to place some of it here and the other part in the panel.
Created attachment 126941 [details] Patch
Attachment 126941 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource From git://git.webkit.org/WebKit 3db9340..6b2655f master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 107692 = 3db9340249163e9ff8eca4792746293bbc60277d r107693 = 980fd80b57e9db12d3998920b813b794c1188c9a r107694 = 6b2655f69a6da87f57db7d8a842ffa5b3b0d73f2 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc M Source/WebKit/chromium/src/WebLayerTreeView.cpp M Source/WebKit/chromium/ChangeLog r107695 = 3ef0e66d03758bef5ec0a5816665dead0f5aee3f (refs/remotes/origin/master) First, rewinding head to replay your work on top of it... Applying: [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets Using index info to reconstruct a base tree... <stdin>:1578: trailing whitespace. <stdin>:1647: trailing whitespace. <stdin>:1657: trailing whitespace. <stdin>:1672: trailing whitespace. return 0; <stdin>:1674: trailing whitespace. warning: squelched 7 whitespace errors warning: 12 lines add whitespace errors. Falling back to patching base and 3-way merge... warning: too many files (created: 168766 deleted: 3), skipping inexact rename detection Auto-merging LayoutTests/ChangeLog CONFLICT (content): Merge conflict in LayoutTests/ChangeLog Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Failed to merge in the changes. Patch failed at 0001 [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 126941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126941&action=review > Source/WebCore/inspector/front-end/ElementsPanel.js:359 > + // We get here for CSS property treeElements, too, so bail out if there is no "nodeName" method. Testing for nodeName is fragile - imagine more tree elements with representedObjects containing nodeName property. You should only call this method for ElementsTreeElements. > Source/WebCore/inspector/front-end/ElementsPanel.js:374 > + treeElement._dimensions = { offsetWidth: properties[0], offsetHeight: properties[1], naturalWidth: properties[2], naturalHeight: properties[3] }; Do not mutate objects that don't belong to you - you should simply do var dimetions here and return them in a callback > Source/WebCore/inspector/front-end/ElementsPanel.js:393 > + object.callFunction(dimensions, setDimensionsData); We now have a returnByValue parameter in callFunctionOn. So that you did not have to JSON.parse and concatenate the string. Simply return an array and use it as array. > Source/WebCore/inspector/front-end/ElementsPanel.js:396 > + WebInspector.RemoteObject.resolveNode(node, "", resolvedNode); Recently, I started using direct flow of callbacks (as in our tests). It is easier to read the code that way. You should consider giving it a try. > Source/WebCore/inspector/front-end/ElementsPanel.js:403 > + if (listItem.treeElement._dimensions) No need to cache - things might change.
Created attachment 126950 [details] Patch
Attachment 126950 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource First, rewinding head to replay your work on top of it... Applying: [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets Using index info to reconstruct a base tree... <stdin>:1578: trailing whitespace. <stdin>:1647: trailing whitespace. <stdin>:1657: trailing whitespace. <stdin>:1672: trailing whitespace. return 0; <stdin>:1674: trailing whitespace. warning: squelched 7 whitespace errors warning: 12 lines add whitespace errors. Falling back to patching base and 3-way merge... warning: too many files (created: 168766 deleted: 3), skipping inexact rename detection Auto-merging LayoutTests/ChangeLog CONFLICT (content): Merge conflict in LayoutTests/ChangeLog Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Auto-merging Tools/ChangeLog CONFLICT (content): Merge conflict in Tools/ChangeLog Failed to merge in the changes. Patch failed at 0001 [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r107705: <http://trac.webkit.org/changeset/107705>