Fix invalid selection produced when dragging mouse outside the SVG text element

third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp

 /*
  * Copyright (C) 2007, 2008 Rob Buis <buis@kde.org>
  * Copyright (C) 2007 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2007 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2009 Google, Inc.  All rights reserved.
  * Copyright (C) 2009 Dirk Schulze <krit@webkit.org>
  * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 458 matching lines @@
     ASSERT(layoutObject);
 
     // FIXME: trying to compute a device space transform at record time is wrong. All clients
     // should be updated to avoid relying on this information, and the method should be removed.
     AffineTransform ctm = deprecatedCalculateTransformToLayer(layoutObject) * SubtreeContentTransformScope::currentContentTransformation();
     ctm.scale(layoutObject->document().frameHost()->deviceScaleFactor());
 
     return narrowPrecisionToFloat(sqrt((pow(ctm.xScale(), 2) + pow(ctm.yScale(), 2)) / 2));
 }
 
+LayoutObject* SVGLayoutSupport::findSelectedLayoutSVGText(LayoutObject* layoutObject, AffineTransform& localToParentTransform, const FloatPoint point)
+{
+    LayoutObject* closestLayoutObject = nullptr;
+    float closestDistance = std::numeric_limits<float>::max();
+    Vector<LayoutObject*> overlappedObjects;
+    for (LayoutObject* child = layoutObject->slowLastChild(); child; child = child->previousSibling()) {
+        if (child->style()->visibility() != VISIBLE)

[fs, 2016/01/15 10:34:16]

This is not correct at this level - 'visibility' can still be 'visible' for any
descendant.

+            continue;
+
+        if (!child->isSVGContainer() && !child->isSVGText())

[fs, 2016/01/15 10:34:16]

"Hidden containers" still return true for isSVGContainer(), so this needs to be
"(isSVGContainer() && !isSVGHiddenContainer())".

+            continue;
+
+        FloatRect bound = child->objectBoundingBox();
+        float distance = bound.squaredDistanceTo(child->localToParentTransform().inverse().mapPoint(point));

[fs, 2016/01/15 10:34:16]

Since you're using the inverse transform you should first check that it is
invertable (if it isn't then the subtree can be skipped.)

+
+        if (distance < closestDistance) {
+            AffineTransform transform;
+
+            if (closestLayoutObject) {
+                transform = localToParentTransform * closestLayoutObject->localToParentTransform();

[fs, 2016/01/15 10:34:16]

No need to transform by |localToParentTransform| since |closestLayoutObject| and
|child| can be brought to the same coordinate space just by applying their
respective localToParentTransform()s (and thus so can their oBBs.)

+                bound = (localToParentTransform * child->localToParentTransform()).mapRect(bound);
+            }
+
+            if (closestLayoutObject && bound.intersects(transform.mapRect(closestLayoutObject->objectBoundingBox()))) {

[fs, 2016/01/15 10:34:16]

Not quite sure why you'd need to check overlap between the two bboxes? If this
new LayoutObject is closer, it seems we'd want to search it first? If both
contain the point, I guess we'd want to search them both (in the order in which
they were initially encountered)? This needs documentation/comments.

+                overlappedObjects.append(child);
+                continue;
+            }
+            overlappedObjects.clear();
+
+            closestLayoutObject = child;
+            closestDistance = distance;
+        }
+    }
+
+    if (closestLayoutObject && !closestLayoutObject->isSVGText()) {

[fs, 2016/01/15 10:34:16]

Use "early-out style" when possible - i.e here you'd do:

if (!closestLayoutObject || closestLayoutObject->isSVGText())
    return closestLayoutObject;
...

+        FloatPoint absolutePoint = closestLayoutObject->localToParentTransform().inverse().mapPoint(point);
+        localToParentTransform = localToParentTransform * closestLayoutObject->localToParentTransform();
+        LayoutObject* result  = findSelectedLayoutSVGText(closestLayoutObject, localToParentTransform, absolutePoint);
+
+        if (!result) {
+            localToParentTransform = localToParentTransform * closestLayoutObject->localToParentTransform().inverse();

[fs, 2016/01/15 10:34:16]

Maintaining the transform like this looks really error-prone... It'd probably be
better to just leave that out entirely - just inverse-transform the point and
work completely in the local space, and then compute the CTM for the object
found in LayoutSVGRoot.

+            for (LayoutObject* overlappedObject : overlappedObjects) {
+                absolutePoint = overlappedObject->localToParentTransform().inverse().mapPoint(point);
+                localToParentTransform = localToParentTransform * overlappedObject->localToParentTransform();
+                result = findSelectedLayoutSVGText(overlappedObject, localToParentTransform, absolutePoint);
+                if (result)
+                    break;
+                localToParentTransform = localToParentTransform * overlappedObject->localToParentTransform().inverse();
+            }
+        }
+
+        closestLayoutObject = result;
+    }
+
+    return closestLayoutObject;
 }
+
+}