Correct LayoutUnit usage in Accessibility code

https://bugs.webkit.org/show_bug.cgi?id=81789

Reviewed by Eric Seidel.

Reverting Accessibility hit testing code back to integers. Accessibility hit tests originate from
the embedder and don't accumulate offsets, so we get nothing from using LayoutUnits, and needlessly
expose them to the embedder.

No new tests. No change in behavior.

* accessibility/AccessibilityListBox.cpp:
(WebCore::AccessibilityListBox::elementAccessibilityHitTest): See above.
* accessibility/AccessibilityListBox.h:
(AccessibilityListBox):
* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::clickPoint): This value is only ever used to display a context menu,
which is always done with integer coordinates.
(WebCore::AccessibilityObject::boundingBoxForQuads): This is a bounding box built from floats. We
don't pixel snap floats, so we return an integer bounding box.
(WebCore::AccessibilityObject::elementAccessibilityHitTest): See above.
(WebCore::AccessibilityObject::scrollToMakeVisible): Pixel snapping the bounding box and simplifying
up the code to position it at (0,0).
* accessibility/AccessibilityObject.h:
(WebCore::AccessibilityObject::accessibilityHitTest): See above.
(AccessibilityObject):
(WebCore::AccessibilityObject::pixelSnappedBoundingBoxRect): Convenience method for embedder callers.
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::visiblePositionForPoint): The point passed in here is comes from
screen coordinates and originates in embedder code. Reverting it to take an integer.
(WebCore::AccessibilityRenderObject::accessibilityImageMapHitTest): See above.
(WebCore::AccessibilityRenderObject::accessibilityHitTest): See above.
* accessibility/AccessibilityRenderObject.h:
(AccessibilityRenderObject):
* accessibility/AccessibilityScrollView.cpp:
(WebCore::AccessibilityScrollView::accessibilityHitTest): See above.
* accessibility/AccessibilityScrollView.h:
(AccessibilityScrollView):
* accessibility/AccessibilitySlider.cpp:
(WebCore::AccessibilitySlider::elementAccessibilityHitTest): See above.
* accessibility/AccessibilitySlider.h:
(AccessibilitySlider):


Canonical link: https://commits.webkit.org/99193@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@111699 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,48 @@
+2012-03-22  Levi Weintraub  <leviw@chromium.org>
+
+        Correct LayoutUnit usage in Accessibility code
+        https://bugs.webkit.org/show_bug.cgi?id=81789
+
+        Reviewed by Eric Seidel.
+
+        Reverting Accessibility hit testing code back to integers. Accessibility hit tests originate from
+        the embedder and don't accumulate offsets, so we get nothing from using LayoutUnits, and needlessly
+        expose them to the embedder.
+
+        No new tests. No change in behavior.
+
+        * accessibility/AccessibilityListBox.cpp:
+        (WebCore::AccessibilityListBox::elementAccessibilityHitTest): See above.
+        * accessibility/AccessibilityListBox.h:
+        (AccessibilityListBox):
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::clickPoint): This value is only ever used to display a context menu,
+        which is always done with integer coordinates.
+        (WebCore::AccessibilityObject::boundingBoxForQuads): This is a bounding box built from floats. We
+        don't pixel snap floats, so we return an integer bounding box.
+        (WebCore::AccessibilityObject::elementAccessibilityHitTest): See above.
+        (WebCore::AccessibilityObject::scrollToMakeVisible): Pixel snapping the bounding box and simplifying
+        up the code to position it at (0,0).
+        * accessibility/AccessibilityObject.h:
+        (WebCore::AccessibilityObject::accessibilityHitTest): See above.
+        (AccessibilityObject):
+        (WebCore::AccessibilityObject::pixelSnappedBoundingBoxRect): Convenience method for embedder callers.
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::visiblePositionForPoint): The point passed in here is comes from
+        screen coordinates and originates in embedder code. Reverting it to take an integer.
+        (WebCore::AccessibilityRenderObject::accessibilityImageMapHitTest): See above.
+        (WebCore::AccessibilityRenderObject::accessibilityHitTest): See above.
+        * accessibility/AccessibilityRenderObject.h:
+        (AccessibilityRenderObject):
+        * accessibility/AccessibilityScrollView.cpp:
+        (WebCore::AccessibilityScrollView::accessibilityHitTest): See above.
+        * accessibility/AccessibilityScrollView.h:
+        (AccessibilityScrollView):
+        * accessibility/AccessibilitySlider.cpp:
+        (WebCore::AccessibilitySlider::elementAccessibilityHitTest): See above.
+        * accessibility/AccessibilitySlider.h:
+        (AccessibilitySlider):
+
 2012-03-21  Ilya Tikhonovsky  <loislo@chromium.org>
 
         Web Inspector: HeapProfiler: Heap snapshot worker has to report the errors to the front-end

Source/WebCore/accessibility/AccessibilityListBox.cpp

@@ -163,7 +163,7 @@ bool AccessibilityListBox::accessibilityIsIgnored() const
     return false;
 }
 
-AccessibilityObject* AccessibilityListBox::elementAccessibilityHitTest(const LayoutPoint& point) const
+AccessibilityObject* AccessibilityListBox::elementAccessibilityHitTest(const IntPoint& point) const
 {
     // the internal HTMLSelectElement methods for returning a listbox option at a point
     // ignore optgroup elements.

Source/WebCore/accessibility/AccessibilityListBox.h

@@ -56,7 +56,7 @@ class AccessibilityListBox : public AccessibilityRenderObject {
 private:    
     AccessibilityObject* listBoxOptionAccessibilityObject(HTMLElement*) const;
     virtual bool accessibilityIsIgnored() const;
-    virtual AccessibilityObject* elementAccessibilityHitTest(const LayoutPoint&) const;
+    virtual AccessibilityObject* elementAccessibilityHitTest(const IntPoint&) const;
 };
 
 } // namespace WebCore

Source/WebCore/accessibility/AccessibilityObject.cpp

@@ -489,25 +489,25 @@ bool AccessibilityObject::isARIAControl(AccessibilityRole ariaRole)
     || ariaRole == ComboBoxRole || ariaRole == SliderRole; 
 }
 
-LayoutPoint AccessibilityObject::clickPoint()
+IntPoint AccessibilityObject::clickPoint()
 {
     LayoutRect rect = elementRect();
-    return LayoutPoint(rect.x() + rect.width() / 2, rect.y() + rect.height() / 2);
+    return roundedIntPoint(LayoutPoint(rect.x() + rect.width() / 2, rect.y() + rect.height() / 2));
 }
 
-LayoutRect AccessibilityObject::boundingBoxForQuads(RenderObject* obj, const Vector<FloatQuad>& quads)
+IntRect AccessibilityObject::boundingBoxForQuads(RenderObject* obj, const Vector<FloatQuad>& quads)
 {
     ASSERT(obj);
     if (!obj)
-        return LayoutRect();
+        return IntRect();
 
     size_t count = quads.size();
     if (!count)
-        return LayoutRect();
+        return IntRect();
 
-    LayoutRect result;
+    IntRect result;
     for (size_t i = 0; i < count; ++i) {
-        LayoutRect r = quads[i].enclosingBoundingBox();
+        IntRect r = quads[i].enclosingBoundingBox();
         if (!r.isEmpty()) {
             if (obj->style()->hasAppearance())
                 obj->theme()->adjustRepaintRect(obj, r);
@@ -1466,7 +1466,7 @@ bool AccessibilityObject::supportsARIALiveRegion() const
     return equalIgnoringCase(liveRegion, "polite") || equalIgnoringCase(liveRegion, "assertive");
 }
 
-AccessibilityObject* AccessibilityObject::elementAccessibilityHitTest(const LayoutPoint& point) const
+AccessibilityObject* AccessibilityObject::elementAccessibilityHitTest(const IntPoint& point) const
 { 
     // Send the hit test back into the sub-frame if necessary.
     if (isAttachment()) {
@@ -1619,8 +1619,8 @@ static int computeBestScrollOffset(int currentScrollOffset,
 
 void AccessibilityObject::scrollToMakeVisible() const
 {
-    IntRect objectRect = boundingBoxRect();
-    objectRect.move(-objectRect.x(), -objectRect.y());
+    IntRect objectRect = pixelSnappedIntRect(boundingBoxRect());
+    objectRect.setLocation(IntPoint());
     scrollToMakeVisibleWithSubFocus(objectRect);
 }
 

Source/WebCore/accessibility/AccessibilityObject.h

@@ -461,9 +461,9 @@ class AccessibilityObject : public RefCounted<AccessibilityObject> {
     virtual void determineARIADropEffects(Vector<String>&) { }
 
     // Called on the root AX object to return the deepest available element.
-    virtual AccessibilityObject* accessibilityHitTest(const LayoutPoint&) const { return 0; }
+    virtual AccessibilityObject* accessibilityHitTest(const IntPoint&) const { return 0; }
     // Called on the AX object after the render tree determines which is the right AccessibilityRenderObject.
-    virtual AccessibilityObject* elementAccessibilityHitTest(const LayoutPoint&) const;
+    virtual AccessibilityObject* elementAccessibilityHitTest(const IntPoint&) const;
 
     virtual AccessibilityObject* focusedUIElement() const;
 
@@ -503,10 +503,11 @@ class AccessibilityObject : public RefCounted<AccessibilityObject> {
     virtual Element* anchorElement() const { return 0; }
     virtual Element* actionElement() const { return 0; }
     virtual LayoutRect boundingBoxRect() const { return LayoutRect(); }
+    IntRect pixelSnappedBoundingBoxRect() const { return pixelSnappedIntRect(boundingBoxRect()); }
     virtual LayoutRect elementRect() const = 0;
     virtual LayoutSize size() const { return elementRect().size(); }
-    virtual LayoutPoint clickPoint();
-    static LayoutRect boundingBoxForQuads(RenderObject*, const Vector<FloatQuad>&);
+    virtual IntPoint clickPoint();
+    static IntRect boundingBoxForQuads(RenderObject*, const Vector<FloatQuad>&);
 
     virtual PlainTextRange selectedTextRange() const { return PlainTextRange(); }
     unsigned selectionStart() const { return selectedTextRange().start; }

Source/WebCore/accessibility/AccessibilityRenderObject.cpp

@@ -2651,7 +2651,7 @@ void AccessibilityRenderObject::setSelectedVisiblePositionRange(const VisiblePos
     }    
 }
 
-VisiblePosition AccessibilityRenderObject::visiblePositionForPoint(const LayoutPoint& point) const
+VisiblePosition AccessibilityRenderObject::visiblePositionForPoint(const IntPoint& point) const
 {
     if (!m_renderer)
         return VisiblePosition();
@@ -2676,7 +2676,7 @@ VisiblePosition AccessibilityRenderObject::visiblePositionForPoint(const LayoutP
     while (1) {
         LayoutPoint ourpoint;
 #if PLATFORM(MAC)
-        ourpoint = frameView->screenToContents(roundedIntPoint(point));
+        ourpoint = frameView->screenToContents(point);
 #else
         ourpoint = point;
 #endif
@@ -2819,7 +2819,7 @@ IntRect AccessibilityRenderObject::doAXBoundsForRange(const PlainTextRange& rang
     return IntRect();
 }
 
-AccessibilityObject* AccessibilityRenderObject::accessibilityImageMapHitTest(HTMLAreaElement* area, const LayoutPoint& point) const
+AccessibilityObject* AccessibilityRenderObject::accessibilityImageMapHitTest(HTMLAreaElement* area, const IntPoint& point) const
 {
     if (!area)
         return 0;
@@ -2855,7 +2855,7 @@ AccessibilityObject* AccessibilityRenderObject::accessibilityHitTest(const IntPo
     Node* node = hitTestResult.innerNode()->shadowAncestorNode();
 
     if (node->hasTagName(areaTag)) 
-        return accessibilityImageMapHitTest(static_cast<HTMLAreaElement*>(node), roundedIntPoint(point));
+        return accessibilityImageMapHitTest(static_cast<HTMLAreaElement*>(node), point);
 
     if (node->hasTagName(optionTag))
         node = static_cast<HTMLOptionElement*>(node)->ownerSelectElement();

Source/WebCore/accessibility/AccessibilityRenderObject.h

@@ -154,7 +154,7 @@ class AccessibilityRenderObject : public AccessibilityObject {
     void updateAccessibilityRole();
 
     // Should be called on the root accessibility object to kick off a hit test.
-    virtual AccessibilityObject* accessibilityHitTest(const LayoutPoint&) const;
+    virtual AccessibilityObject* accessibilityHitTest(const IntPoint&) const;
 
     virtual Element* actionElement() const;
     Element* mouseButtonListener() const;
@@ -239,7 +239,7 @@ class AccessibilityRenderObject : public AccessibilityObject {
     virtual bool isARIAGrabbed();
     virtual void determineARIADropEffects(Vector<String>&);
 
-    virtual VisiblePosition visiblePositionForPoint(const LayoutPoint&) const;
+    virtual VisiblePosition visiblePositionForPoint(const IntPoint&) const;
     virtual VisiblePosition visiblePositionForIndex(unsigned indexValue, bool lastIndexOK) const;    
     virtual int index(const VisiblePosition&) const;
 

Source/WebCore/accessibility/AccessibilityScrollView.cpp

@@ -163,7 +163,7 @@ AccessibilityObject* AccessibilityScrollView::webAreaObject() const
     return axObjectCache()->getOrCreate(doc->renderer());
 }
 
-AccessibilityObject* AccessibilityScrollView::accessibilityHitTest(const LayoutPoint& point) const
+AccessibilityObject* AccessibilityScrollView::accessibilityHitTest(const IntPoint& point) const
 {
     AccessibilityObject* webArea = webAreaObject();
     if (!webArea)

Source/WebCore/accessibility/AccessibilityScrollView.h

@@ -57,7 +57,7 @@ class AccessibilityScrollView : public AccessibilityObject {
     virtual AccessibilityObject* scrollBar(AccessibilityOrientation);
     virtual void addChildren();
     virtual void clearChildren();
-    virtual AccessibilityObject* accessibilityHitTest(const LayoutPoint&) const;
+    virtual AccessibilityObject* accessibilityHitTest(const IntPoint&) const;
     virtual void updateChildrenIfNecessary();
     virtual void setNeedsToUpdateChildren() { m_childrenDirty = true; }
     void updateScrollbars();

Source/WebCore/accessibility/AccessibilitySlider.cpp

@@ -102,7 +102,7 @@ const AtomicString& AccessibilitySlider::getAttribute(const QualifiedName& attri
     return element()->getAttribute(attribute);
 }
 
-AccessibilityObject* AccessibilitySlider::elementAccessibilityHitTest(const LayoutPoint& point) const
+AccessibilityObject* AccessibilitySlider::elementAccessibilityHitTest(const IntPoint& point) const
 {
     if (m_children.size()) {
         ASSERT(m_children.size() == 1);

Source/WebCore/accessibility/AccessibilitySlider.h

@@ -48,7 +48,7 @@ class AccessibilitySlider : public AccessibilityRenderObject {
 private:
     HTMLInputElement* element() const;
     virtual bool accessibilityIsIgnored() const;
-    virtual AccessibilityObject* elementAccessibilityHitTest(const LayoutPoint&) const;
+    virtual AccessibilityObject* elementAccessibilityHitTest(const IntPoint&) const;
 
     virtual AccessibilityRole roleValue() const { return SliderRole; }    
     virtual bool isSlider() const { return true; }