RESOLVED FIXED Bug 13981
<br> prevents click handler from firing 
https://bugs.webkit.org/show_bug.cgi?id=13981
Summary <br> prevents click handler from firing
Antoine Quint
Reported 2007-06-03 05:42:43 PDT
A click event handler on an SVG element positioned after an XHTML <br /> element does not get fired. Removing the <br /> makes the event handler work. See test case.
Attachments
Test case for <br> before an <svg> element (624 bytes, application/xml)
2007-06-03 05:43 PDT, Antoine Quint 		no flags
Details
Render tree comparisons (left=<p/><br/><svg>, right=<p/><br/><div/><svg>) (103.59 KB, image/png)
2007-06-04 11:33 PDT, Eric Seidel (no email) 		no flags
Details
render tree for broken case (45.67 KB, image/png)
2007-06-04 21:45 PDT, Eric Seidel (no email) 		no flags
Details
First attempt (27.26 KB, patch)
2007-06-17 05:54 PDT, Rob Buis 		eric: review-
DetailsFormatted DiffDiff
Addressing Eric's issue (27.53 KB, patch)
2007-06-19 00:34 PDT, Rob Buis 		oliver: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antoine Quint
Comment 1 2007-06-03 05:43:28 PDT
Created attachment 14847 [details] Test case for <br> before an <svg> element
Eric Seidel (no email)
Comment 2 2007-06-04 11:23:56 PDT
Well, clearly the <br> is overlapping with the <svg> and capturing the click.
Eric Seidel (no email)
Comment 3 2007-06-04 11:27:25 PDT
If you start the doc <body><br /><svg> the problem does not occur. So it seems to be an interaction between the <p> and the <br> which produces this <br /> overlap with the <svg>
Eric Seidel (no email)
Comment 4 2007-06-04 11:29:10 PDT
More info: Using a <div> instead of an SVG does not show the bug. I expect that RenderSVGContainer is just not implementing some render tree method correctly.
Eric Seidel (no email)
Comment 5 2007-06-04 11:33:40 PDT
Created attachment 14854 [details] Render tree comparisons (left=<p/><br/><svg>, right=<p/><br/><div/><svg>)
Eric Seidel (no email)
Comment 6 2007-06-04 11:34:42 PDT
Hyatt might just know off the top of his head what's up here.
Eric Seidel (no email)
Comment 7 2007-06-04 21:45:55 PDT
Created attachment 14857 [details] render tree for broken case
Eric Seidel (no email)
Comment 8 2007-06-04 22:24:12 PDT
Well, I think we're applying the _tx, _ty twice. Possibly because we're doing it once in nodeAtPoint and a second time using absoluteTransform() (from inside mapAbsolutePointToLocal()). IMO the best way to go about fixing this is to cleanly divide the SVG render tree from the HTML render tree by finally separating out the "in HTML" code from RenderSVGContainer into a RenderSVGRoot class or similar. http://bugs.webkit.org/show_bug.cgi?id=12207 covers doing such. There might also be an easy way to fix this, I'm not sure. absoluteTransform() application is necessary to handle rotation etc. applying _tx, _ty is necessary due to handle scrolling. Again, I think this type of bug mostly just occurs due to the strange intermixing of HTML and SVG way of doing things in RenderSVGContainer (since RSC can be inside either html or svg content).
Rob Buis
Comment 9 2007-06-17 05:54:57 PDT
Created attachment 15084 [details] First attempt This should fix hit testing. Also the rects in the window inspector should be more accurate for svg renderers now. Cheers, Rob.
Eric Seidel (no email)
Comment 10 2007-06-18 10:08:50 PDT
Comment on attachment 15084 [details] First attempt This line needs to be made more understandable: - if (child->nodeAtPoint(request, result, _x, _y, _tx, _ty, hitTestAction)) { + if (child->nodeAtPoint(request, result, _x - (_tx - ctm.e()), _y - (_ty - ctm.f()), 0, 0, hitTestAction)) { by breaking it out into more lines with better names. The code otherwise looks fine.
Rob Buis
Comment 11 2007-06-19 00:34:59 PDT
Created attachment 15114 [details] Addressing Eric's issue This one has a clear name for the calculated scroll offets. Cheers, Rob.
Oliver Hunt
Comment 12 2007-06-19 00:46:25 PDT
Comment on attachment 15114 [details] Addressing Eric's issue looks fine
Rob Buis
Comment 13 2007-06-19 02:16:20 PDT
Landed in r23583.
Note You need to log in before you can comment on or make changes to this bug.