Created attachment 48347 [details] test case The onclick javascript handler does not fire under some common conditions with SVG elements. I have created a minimal self-contained test case which is attached. The test case displays two SVG rectangles. On the left rectangle, using the currently released Safari and today's nightly build, no click is detected. On the right rectangle, the click is detected. The only difference is the transform on the left rectangle element. The transform correctly adjusts the display, but interferes incorrectly with click detection. Both clicks are detected correctly in Firefox. Chrome has the same behavior as Safari.
This is still broken in Safari 4.0.5.
I tested it with another coordinate base than 'cm' and it worked. So maybe this is not a problem of hittesting itself. Also do the results differ between Safari Chrome and WebKitGtk, but shouldn't. Maybe the coordinates get to small during the mapping to the local transform, so that this is caused by rounding problems. Needs further testing.
Created attachment 125349 [details] Patch
Attachment 125349 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/svg/..." exit_code: 1 LayoutTests/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Source/WebCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Source/WebCore/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Total errors found: 4 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 125349 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125349&action=review r- because of style. > Source/WebCore/ChangeLog:8 > + Use FloatPoints in RenderSVGRoot::nodeAtPoint() when converting the incoming point to local coordinates. No tabs at all please. > LayoutTests/ChangeLog:8 > + Checks that elementFromPoint() works correctly with SVG elements defined within a 1x1 viewbox. Ditto. > LayoutTests/svg/hittest/script-tests/svg-floating-point-transform.js:6 > + {x:1, y:1}, > + {x:1, y:399}, > + {x:399, y:1}, Indentation should be 4 spaces. For tests as well. Also make a space between ':' and the following number.
Created attachment 125353 [details] Patch
Comment on attachment 125353 [details] Patch Attachment 125353 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11416407 New failing tests: svg/text/select-textLength-spacingAndGlyphs-squeeze-3.svg svg/text/select-textLength-spacing-stretch-3.svg svg/text/select-textLength-spacingAndGlyphs-squeeze-4.svg svg/text/select-textLength-spacingAndGlyphs-squeeze-2.svg svg/text/select-x-list-2.svg svg/text/select-x-list-with-tspans-2.svg
Comment on attachment 125353 [details] Patch r=me. I don't see a relation to the bugs on the bug report and the detailed list of the build bot. I assume that the build list is the one we should look at. And I don't sees a failing SVG test. To the test: you shouldn't generate the html file on yourself. This gets generated by a script called Tools/Scripts/make-script-test-wrappers (you have to call it manually. It is located in Tools/Scripts). Instead you should create the elements dynamically in your script. Would be great if you can change the test. Please see also at LayoutTests/svg/dynamic-updates/ for examples.
(In reply to comment #8) > (From update of attachment 125353 [details]) > r=me. The r=me was wrong. Sorry, not sure why it is in the text.
Created attachment 127444 [details] Patch
I've uploaded changes to the expected txt and png files for the select-textLength-spacing-squeeze-2 test for the mac platform only. Not sure how to cause those files to be generated for the other platforms.
Comment on attachment 127444 [details] Patch Attachment 127444 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11542186 New failing tests: svg/text/select-textLength-spacing-squeeze-2.svg svg/hittest/svg-small-viewbox.xhtml
Comment on attachment 127444 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127444&action=review The failing tests need to be marked in platform/chromium/test_expectations.txt. BUGWK<YOURBUG>: svg/your/file.svg = IMAGE+TEXT or = IMAGE (depending on if you expect image+text to fail, or only the image). The usual process for generating baselines is: * Amend chromium expectations, to paint the cr-linux EWS green * Include test results generated on Lion, inclduing pixel test baselines, if needed * Land your patch, wait for Gtk/Qt/Win to cycle, then use "webkit-patch rebaseline" to grab the new results from these platforms, then land them as follow-up patch. r- because the test won't work in trunk anymore, due recent changes: > LayoutTests/svg/hittest/script-tests/svg-small-viewbox.js:54 > +startTest(rootSVGElement, 100, 100); The testcase is outdated, coded against a version before we switched to the repaint.js harness. I fear you have to upate it again. Look at how svg/<xxx>/script-tests/foobar.js tests look now in trunk (no more startTest, executeTest renamed to repaintTest). You also have to include repaint.js in your .html file now.
Created attachment 127671 [details] Patch
I've revised the new test, svg/hittest/svg-small-viewbox.xhtml, to look as much the other hittests as possible. It no longer depends on any shared scripts, and it's just one file.
Comment on attachment 127671 [details] Patch Clearing flags on attachment: 127671 Committed r108857: <http://trac.webkit.org/changeset/108857>
All reviewed patches have been landed. Closing bug.
Committed r114413: <http://trac.webkit.org/changeset/114413>