Bug 12201 - Fix image dragging issues with http://www.carto.net/papers/svg/gui/scrollbar/index.svg
Summary: Fix image dragging issues with http://www.carto.net/papers/svg/gui/scrollbar/...
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL: http://www.carto.net/papers/svg/gui/s...
Depends on:
Reported: 2007-01-10 11:58 PST by Rob Buis
Modified: 2007-01-11 04:41 PST (History)
0 users

See Also:

First attempt (52.36 KB, patch)
2007-01-10 12:13 PST, Rob Buis
eric: review-
Details | Formatted Diff | Diff
Improved patch (51.16 KB, patch)
2007-01-11 00:42 PST, Rob Buis
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2007-01-10 11:58:39 PST
The testcase shows problems with hit testing of images. In this case the image above seems selected but the image below is dragged mistakenly.
Comment 1 Rob Buis 2007-01-10 12:13:21 PST
Created attachment 12349 [details]
First  attempt

This should not be so hard a patch to review... AFAICS all new test results are improvements regarding positioning info and pixel tests are same as before.

Comment 2 Eric Seidel (no email) 2007-01-10 15:59:44 PST
Comment on attachment 12349 [details]
First  attempt

Glad to see this one fixed!

Bleh.  temp and ret are really bad variable names.  We should use something more descriptive.

This extra multiply is not necessary:
-        return getAspectRatio(viewBox(), viewportRect);
+        ret = getAspectRatio(viewBox(), viewportRect) * ret;

ret should be the identity there.

I'm not 100% sure this is correct.  It probably is, but I'd like to you tell me more about it over IRC.
Comment 3 Rob Buis 2007-01-11 00:42:08 PST
Created attachment 12356 [details]
Improved patch

Fixing Eric's issues.

Comment 4 Eric Seidel (no email) 2007-01-11 01:01:15 PST
Comment on attachment 12356 [details]
Improved patch

This looks good.

Please add a comment about viewportTransform() not including translation.  As you mentioned on IRC, this is confusing due to <svg> and <g> both using RenderSVGContainer (and one of them having only localTransform and one having only viewportTransform())

I filed:
about splitting them up.
Comment 5 David Kilzer (:ddkilzer) 2007-01-11 04:41:21 PST
Landed by rwlbuis in r18760.