60630 2011-05-11 08:50:32 -0700 Web Inspector: hovering over linkified nodes in the UI does not highlight elements on the page. 2011-06-21 01:43:52 -0700 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 0 pfeldman pfeldman apavlov bweinstein joepeck keishi loislo pfeldman pmuellr rik timothy yurys oldest_to_newest 401996 0 pfeldman 2011-05-11 08:50:32 -0700 Patch to follow. 401998 1 93125 pfeldman 2011-05-11 08:55:12 -0700 Created attachment 93125 Patch 402141 2 93125 yurys 2011-05-11 11:56:37 -0700 Comment on attachment 93125 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? 402161 3 93125 apavlov 2011-05-11 12:24:16 -0700 Comment on attachment 93125 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. 402606 4 yurys 2011-05-11 20:38:49 -0700 (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. 402643 5 93125 yurys 2011-05-11 21:47:40 -0700 Comment on attachment 93125 Patch r+ since it's a regression. The method should be split in a separate change. 423093 6 pfeldman 2011-06-18 02:27:31 -0700 I just checken on a canary and it does not seem to work. Reopening. I'll rebase and land. 424345 7 93125 pfeldman 2011-06-21 01:43:43 -0700 Comment on attachment 93125 Patch Clearing flags on attachment: 93125 Committed r89342: <http://trac.webkit.org/changeset/89342> 424346 8 pfeldman 2011-06-21 01:43:52 -0700 All reviewed patches have been landed. Closing bug. 93125 2011-05-11 08:55:12 -0700 2011-06-21 01:43:43 -0700 Patch bug-60630-20110511085511.patch text/plain 1579 pfeldman U3VidmVyc2lvbiBSZXZpc2lvbjogODYwMjcKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCAzNjA5OGVjZDdlMzlkZTY0 NjMwNzUxZjdiMmQ0OTQxZmFmM2EyMjBkLi42YjA1YWNhMTIzMjBiZjU1YzM0MGU3NmMwOWI4YzJj ZWU5MzE3NDIxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTEtMDUtMTEgIFBhdmVs IEZlbGRtYW4gIDxwZmVsZG1hbkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogaG92ZXJpbmcgb3ZlciBs aW5raWZpZWQgbm9kZXMKKyAgICAgICAgaW4gdGhlIFVJIGRvZXMgbm90IGhpZ2hsaWdodCBlbGVt ZW50cyBvbiB0aGUgcGFnZS4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTYwNjMwCisKKyAgICAgICAgKiBpbnNwZWN0b3IvZnJvbnQtZW5kL2luc3BlY3Rv ci5qczoKKyAgICAgICAgKFdlYkluc3BlY3Rvci53aXJlRWxlbWVudFdpdGhET01Ob2RlKToKKwog MjAxMS0wNS0wOCAgUGF2ZWwgRmVsZG1hbiAgPHBmZWxkbWFuQGNocm9taXVtLm9yZz4KIAogICAg ICAgICBOb3QgcmV2aWV3ZWQ6IGlnbm9yZSAwIHJlc3BvbnNlcyBpbiB0aGUgaW5zcGVjdG9yIG5l dHdvcmsgaW5zdHJ1bWVudGF0aW9uLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaW5zcGVj dG9yL2Zyb250LWVuZC9pbnNwZWN0b3IuanMgYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvZnJv bnQtZW5kL2luc3BlY3Rvci5qcwppbmRleCBkYWNiNWNmMjMxZjIxYWRkN2IyMDBjM2JkNzAyYWI0 YTZmYWFjMWQ1Li5iMGY2MzBjOTY4MGNlYTM5ZGI5NTVlYmViNzNhZGUxNjNjYmEzYzU0IDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL2luc3BlY3Rvci5qcwor KysgYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL2luc3BlY3Rvci5qcwpAQCAt MzE5LDcgKzMxOSw3IEBAIHZhciBXZWJJbnNwZWN0b3IgPSB7CiAgICAgd2lyZUVsZW1lbnRXaXRo RE9NTm9kZTogZnVuY3Rpb24oZWxlbWVudCwgbm9kZUlkKQogICAgIHsKICAgICAgICAgZWxlbWVu dC5hZGRFdmVudExpc3RlbmVyKCJjbGljayIsIHRoaXMuX3VwZGF0ZUZvY3VzZWROb2RlLmJpbmQo dGhpcywgbm9kZUlkKSwgZmFsc2UpOwotICAgICAgICBlbGVtZW50LmFkZEV2ZW50TGlzdGVuZXIo Im1vdXNlb3ZlciIsIHRoaXMuaGlnaGxpZ2h0RE9NTm9kZS5iaW5kKHRoaXMsIG5vZGVJZCksIGZh bHNlKTsKKyAgICAgICAgZWxlbWVudC5hZGRFdmVudExpc3RlbmVyKCJtb3VzZW92ZXIiLCB0aGlz LmhpZ2hsaWdodERPTU5vZGUuYmluZCh0aGlzLCBub2RlSWQsICJhbGwiKSwgZmFsc2UpOwogICAg ICAgICBlbGVtZW50LmFkZEV2ZW50TGlzdGVuZXIoIm1vdXNlb3V0IiwgdGhpcy5oaWdobGlnaHRE T01Ob2RlLmJpbmQodGhpcywgMCksIGZhbHNlKTsKICAgICB9LAogCg==