Last Comment Bug 693551 - removeChild(): unexpected title tooltip after removing SVG element at mouse position
: removeChild(): unexpected title tooltip after removing SVG element at mouse p...
Status: RESOLVED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 11
Assigned To: Robert Longson
:
:
Mentors:
Depends on:
Blocks: 601091 639945 664058
  Show dependency treegraph
 
Reported: 2011-10-11 03:00 PDT by yvoschu
Modified: 2012-05-20 20:35 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
testcase (134 bytes, image/svg+xml)
2011-10-11 03:00 PDT, yvoschu
no flags Details
patch (1.76 KB, patch)
2011-12-14 06:25 PST, Robert Longson
no flags Details | Diff | Splinter Review
address review comment (2.36 KB, patch)
2011-12-15 07:28 PST, Robert Longson
dao+bmo: review+
Details | Diff | Splinter Review

Description yvoschu 2011-10-11 03:00:46 PDT
Created attachment 566159 [details]
testcase

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0.1) Gecko/20100101 Firefox/7.0.1
Build ID: 20110928134238

Steps to reproduce:

Tested with Firefox 7.0.1 and current Nightly 10.0a1 on Windows:
1.) open a file (e.g. the attached one) containing code like this
<circle cx="100" cy="100" r="50" onclick="this.parentNode.removeChild(this);" />
2.) move mouse over circle and click left mouse button (don't stop moving before clicking!)


Actual results:

A title tooltip is shown that contains the text of the last tooltip, even from an other browser tab. If it is the first title tooltip after startup, the tooltip is shown without text.

AFAICS this doesn't happen with HTML elements or if the mouse was not moved right before clicking.
Comment 1 Alice0775 White 2011-10-11 08:18:03 PDT
Confirmed on 
http://hg.mozilla.org/mozilla-central/rev/e9c620a5c85f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111010 Firefox/10.0a1 ID:20111010031016

Regression window(cached m-c hourly),
Works:
http://hg.mozilla.org/mozilla-central/rev/7898841a922a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110630 Firefox/7.0a1 ID:20110702004346
Fails:
http://hg.mozilla.org/mozilla-central/rev/7b44ed36faf3
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110702 Firefox/7.0a1 ID:20110702022656
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7898841a922a&tochange=7b44ed36faf3

Regression window(cached m-i hourly),
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/4a7f8e181dd0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110630 Firefox/7.0a1 ID:20110701101456
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/eb5866601f88
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110701 Firefox/7.0a1 ID:20110701132816
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4a7f8e181dd0&tochange=eb5866601f88

Triggered by:
74635b831e9e	Robert Longson — Bug 639945 - tooltips are not displayed on inline svg elements. r=dao,bzbarsky
Comment 2 Adam Moore 2011-12-13 11:26:24 PST
I came across this bug using FF8 in the wild when viewing a svg chart in one tab -- the tooltip showed data from another tab, which looks a little scary from a security perspective.
Comment 3 Daniel Holbert [:dholbert] 2011-12-13 11:31:16 PST
Updating platform, as I can reproduce in current nightly on linux:
  Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111212 Firefox/11.0a1
though I needed this additional step in comment 0's STR:
 0. Hover over any link with a tooltip (e.g. this one: attachment 566159 [details] )
Comment 4 Robert Longson 2011-12-13 11:41:16 PST
(In reply to Adam Moore from comment #2)
> I came across this bug using FF8 in the wild when viewing a svg chart in one
> tab -- the tooltip showed data from another tab, which looks a little scary
> from a security perspective.

Not really, the page itself can't read the tooltip data.
Comment 5 Adam Moore 2011-12-13 11:41:53 PST
But how does the typical user know this?
Comment 6 Daniel Holbert [:dholbert] 2011-12-13 12:06:45 PST
> which looks a little scary from a security perspective.

Agreed that it could look scary to a typical user, even if it's not actually dangerous.  (The appearance of insecurity is worth avoiding. :) )
Comment 7 Robert Longson 2011-12-14 06:25:41 PST
Created attachment 581618 [details] [diff] [review]
patch

I don't know how to write a testcase as the existing case relies on the content being removed as the tooltip appears. The c++ embedding tooltip code already has a null check so it doesn't need to change.
Comment 8 Robert Longson 2011-12-14 06:26:25 PST
Note the simplification in the first part of the patch is covered by the existing testcase.
Comment 9 Dão Gottwald [:dao] 2011-12-15 07:00:26 PST
Comment on attachment 581618 [details] [diff] [review]
patch

How about adding document.compareDocumentPosition(tipElement) == document.DOCUMENT_POSITION_DISCONNECTED right at the beginning of FillInHTMLTooltip? Could !tipElement.ownerDocument already be an (unsuccessful) attempt to do this?
Comment 10 Robert Longson 2011-12-15 07:12:12 PST
(In reply to Dão Gottwald [:dao] from comment #9)
> 
> How about adding document.compareDocumentPosition(tipElement) ==
> document.DOCUMENT_POSITION_DISCONNECTED right at the beginning of
> FillInHTMLTooltip? Could !tipElement.ownerDocument already be an
> (unsuccessful) attempt to do this?

That comes from bug 520729
Comment 11 Dão Gottwald [:dao] 2011-12-15 07:17:10 PST
Ok, so that isn't broken. It would have helped if Ehsan had followed the "Maybe add a comment here that explains what cases this is handling (tipElement is a document)" suggestion.

My example code however is broken... document needs to be tipElement.ownerDocument.
Comment 12 Robert Longson 2011-12-15 07:23:39 PST
Ehsan did do that on the comment line above the code.
Comment 13 Robert Longson 2011-12-15 07:28:59 PST
Created attachment 581959 [details] [diff] [review]
address review comment
Comment 15 Matt Brubeck (:mbrubeck) 2011-12-17 09:35:10 PST
https://hg.mozilla.org/mozilla-central/rev/d9e8512aa9d4
Comment 16 Alex Keybl [:akeybl] 2012-01-05 13:24:01 PST
This isn't a regression from 9, so unless we know of significant fallout, no need to uplift to FF10.
Comment 17 yvoschu 2012-01-06 06:13:31 PST
(In reply to Matt Brubeck (:mbrubeck) from comment #15)
> https://hg.mozilla.org/mozilla-central/rev/d9e8512aa9d4

When does the fix show up in the nightly builds? The issue is not fixed in 12.0a1 (2012-01-05).
Comment 18 Alice0775 White 2012-01-06 06:27:54 PST
Confirmed, this does not fixed on 
http://hg.mozilla.org/mozilla-central/rev/5413ba3ed406
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120105 Firefox/12.0a1 ID:20120105083933
and
http://hg.mozilla.org/releases/mozilla-aurora/rev/e03d758c3c08
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a2) Gecko/20120105 Firefox/11.0a2 ID:20120105042010

REOPENED
Comment 19 Alice0775 White 2012-01-06 06:50:29 PST
I think this is related Bug 715882
Comment 20 Alice0775 White 2012-01-06 06:52:10 PST
Sorry
s/I think this is related Bug 715882/I think this is related Bug 664058/
Comment 21 Robert Longson 2012-01-06 11:46:31 PST
Testcase in the bug works for me and the patch has not been backed out so the correct resolution is still fixed. If you still have issues raise another bug, this one is done.
Comment 22 Alice0775 White 2012-01-06 12:01:21 PST
(In reply to Robert Longson from comment #21)
> Testcase in the bug works for me and the patch has not been backed out so
> the correct resolution is still fixed. If you still have issues raise
> another bug, this one is done.

filed Bug 715999

Note You need to log in before you can comment on or make changes to this bug.