Summary: | Web Inspector: yellow on-hover pop-up won't go if another pane asynchronously opens | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Peter Rybin <prybin> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Mirela <mbudaes> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, mbudaes, mibalan, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Peter Rybin
2012-09-19 10:41:47 PDT
I will try looking into this bug. Created attachment 165388 [details]
Add mouseout event listener on popover element.
Comment on attachment 165388 [details] Add mouseout event listener on popover element. View in context: https://bugs.webkit.org/attachment.cgi?id=165388&action=review > Source/WebCore/ChangeLog:3 > + Fix for bug 97210 Please remove this auto-added line > Source/WebCore/inspector/front-end/Popover.js:242 > + if (event.target === this._hoverElement || WebKit has no limit for the line length, so the entire condition can be placed on a single line. And we definitely do not observe closing parentheses on a separate line. > Source/WebCore/inspector/front-end/Popover.js:243 > + (this.isPopoverVisible() && !event.toElement.isSelfOrDescendant(this._popover._contentDiv)) IIRC, event.toElement can be null/undefined if you are moving the pointer outside the browser window quickly (which is confirmed by http://trac.webkit.org/browser/trunk/Source/WebCore/dom/MouseEvent.cpp#L137). Also, "toElement" is an MSIE extension. The spec'ed way is "event.relatedTarget". Created attachment 165556 [details]
Update the patch based on the review.
Comment on attachment 165556 [details] Update the patch based on the review. View in context: https://bugs.webkit.org/attachment.cgi?id=165556&action=review > Source/WebCore/ChangeLog:3 > + Fix for bug 97210 Hmm, please remove this auto-generated line... > Source/WebCore/inspector/front-end/Popover.js:242 > + if (event.target === this._hoverElement || (this.isPopoverVisible() && !event.relatedTarget.isSelfOrDescendant(this._popover._contentDiv))) Please check event.relatedTarget for null-ity to avoid throwing a JS exception. Created attachment 165619 [details]
New patch update
Comment on attachment 165619 [details] New patch update View in context: https://bugs.webkit.org/attachment.cgi?id=165619&action=review Two small nits to fix, and the patch will be ready to land > Source/WebCore/inspector/front-end/Popover.js:244 > + && event.relatedTarget !== null WebKit coding style guidelines require that null/0/undefined/whatever checks be performed by casting to a bool (e.g. ... && event.relatedTarget && ...) unless we need to disambiguate the actual value (e.g. null vs. undefined vs. "" vs. 0) - see http://www.webkit.org/coding/coding-style.html#zero-comparison > Source/WebCore/inspector/front-end/Popover.js:246 > + if (event.target === this._hoverElement || isPopoverMouseOut ) Extraneous whitespace before ')' Created attachment 165621 [details]
Address review comments
Comment on attachment 165621 [details]
Address review comments
r=me
Comment on attachment 165621 [details] Address review comments Clearing flags on attachment: 165621 Committed r129516: <http://trac.webkit.org/changeset/129516> All reviewed patches have been landed. Closing bug. |