Patch to follow.
Created attachment 93125 [details] Patch
Comment on attachment 93125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93125&action=review > Source/WebCore/inspector/front-end/inspector.js:322 > + element.addEventListener("mouseover", this.highlightDOMNode.bind(this, nodeId, "all"), false); How does it differ from the current behavior? Looking at inspector.js:308 I see this: DOMAgent.highlightNode(nodeId, mode || "all"); which is equivalent to passing "all" explicitely. Should we require mode parameter?
Comment on attachment 93125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93125&action=review >> Source/WebCore/inspector/front-end/inspector.js:322 >> + element.addEventListener("mouseover", this.highlightDOMNode.bind(this, nodeId, "all"), false); > > How does it differ from the current behavior? Looking at inspector.js:308 I see this: > DOMAgent.highlightNode(nodeId, mode || "all"); which is equivalent to passing "all" explicitely. Should we require mode parameter? In the case of a bound event handler, the mouse event becomes the second parameter which is far from "all". We should doubtfully require the second parameter, as highlightDOMNode(0) results in hiding the highlight (and having the second "mode" parameter is awkward in this case.) Incidentally, I came up with the same solution today while rewriting the StylesSidebarPane.
(In reply to comment #3) > (From update of attachment 93125 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=93125&action=review > > >> Source/WebCore/inspector/front-end/inspector.js:322 > >> + element.addEventListener("mouseover", this.highlightDOMNode.bind(this, nodeId, "all"), false); > > > > How does it differ from the current behavior? Looking at inspector.js:308 I see this: > > DOMAgent.highlightNode(nodeId, mode || "all"); which is equivalent to passing "all" explicitely. Should we require mode parameter? > > In the case of a bound event handler, the mouse event becomes the second parameter which is far from "all". We should doubtfully require the second parameter, as highlightDOMNode(0) results in hiding the highlight (and having the second "mode" parameter is awkward in this case.) Sounds like the method needs to be split into highlightXXX and hideHighlightXX.
Comment on attachment 93125 [details] Patch r+ since it's a regression. The method should be split in a separate change.
I just checken on a canary and it does not seem to work. Reopening. I'll rebase and land.
Comment on attachment 93125 [details] Patch Clearing flags on attachment: 93125 Committed r89342: <http://trac.webkit.org/changeset/89342>
All reviewed patches have been landed. Closing bug.