Title: [111699] trunk/Source/WebCore
Revision
111699
Author
le...@chromium.org
Date
2012-03-22 08:07:02 -0700 (Thu, 22 Mar 2012)

Log Message

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):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (111698 => 111699)


--- trunk/Source/WebCore/ChangeLog	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/ChangeLog	2012-03-22 15:07:02 UTC (rev 111699)
@@ -1,3 +1,48 @@
+2012-03-22  Levi Weintraub  <le...@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  <loi...@chromium.org>
 
         Web Inspector: HeapProfiler: Heap snapshot worker has to report the errors to the front-end

Modified: trunk/Source/WebCore/accessibility/AccessibilityListBox.cpp (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilityListBox.cpp	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilityListBox.cpp	2012-03-22 15:07:02 UTC (rev 111699)
@@ -163,7 +163,7 @@
     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.

Modified: trunk/Source/WebCore/accessibility/AccessibilityListBox.h (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilityListBox.h	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilityListBox.h	2012-03-22 15:07:02 UTC (rev 111699)
@@ -56,7 +56,7 @@
 private:    
     AccessibilityObject* listBoxOptionAccessibilityObject(HTMLElement*) const;
     virtual bool accessibilityIsIgnored() const;
-    virtual AccessibilityObject* elementAccessibilityHitTest(const LayoutPoint&) const;
+    virtual AccessibilityObject* elementAccessibilityHitTest(const IntPoint&) const;
 };
     
 } // namespace WebCore

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2012-03-22 15:07:02 UTC (rev 111699)
@@ -489,25 +489,25 @@
     || 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 @@
     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 @@
 
 void AccessibilityObject::scrollToMakeVisible() const
 {
-    IntRect objectRect = boundingBoxRect();
-    objectRect.move(-objectRect.x(), -objectRect.y());
+    IntRect objectRect = pixelSnappedIntRect(boundingBoxRect());
+    objectRect.setLocation(IntPoint());
     scrollToMakeVisibleWithSubFocus(objectRect);
 }
 

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.h (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.h	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.h	2012-03-22 15:07:02 UTC (rev 111699)
@@ -461,9 +461,9 @@
     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 @@
     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; }

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2012-03-22 15:07:02 UTC (rev 111699)
@@ -2651,7 +2651,7 @@
     }    
 }
 
-VisiblePosition AccessibilityRenderObject::visiblePositionForPoint(const LayoutPoint& point) const
+VisiblePosition AccessibilityRenderObject::visiblePositionForPoint(const IntPoint& point) const
 {
     if (!m_renderer)
         return VisiblePosition();
@@ -2676,7 +2676,7 @@
     while (1) {
         LayoutPoint ourpoint;
 #if PLATFORM(MAC)
-        ourpoint = frameView->screenToContents(roundedIntPoint(point));
+        ourpoint = frameView->screenToContents(point);
 #else
         ourpoint = point;
 #endif
@@ -2819,7 +2819,7 @@
     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 @@
     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();

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2012-03-22 15:07:02 UTC (rev 111699)
@@ -154,7 +154,7 @@
     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 @@
     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;
 

Modified: trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp	2012-03-22 15:07:02 UTC (rev 111699)
@@ -163,7 +163,7 @@
     return axObjectCache()->getOrCreate(doc->renderer());
 }
 
-AccessibilityObject* AccessibilityScrollView::accessibilityHitTest(const LayoutPoint& point) const
+AccessibilityObject* AccessibilityScrollView::accessibilityHitTest(const IntPoint& point) const
 {
     AccessibilityObject* webArea = webAreaObject();
     if (!webArea)

Modified: trunk/Source/WebCore/accessibility/AccessibilityScrollView.h (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilityScrollView.h	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilityScrollView.h	2012-03-22 15:07:02 UTC (rev 111699)
@@ -57,7 +57,7 @@
     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();

Modified: trunk/Source/WebCore/accessibility/AccessibilitySlider.cpp (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilitySlider.cpp	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilitySlider.cpp	2012-03-22 15:07:02 UTC (rev 111699)
@@ -102,7 +102,7 @@
     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);

Modified: trunk/Source/WebCore/accessibility/AccessibilitySlider.h (111698 => 111699)


--- trunk/Source/WebCore/accessibility/AccessibilitySlider.h	2012-03-22 15:05:41 UTC (rev 111698)
+++ trunk/Source/WebCore/accessibility/AccessibilitySlider.h	2012-03-22 15:07:02 UTC (rev 111699)
@@ -48,7 +48,7 @@
 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; }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to