Changeset 150783 in webkit

Timestamp:

May 27, 2013 4:53:47 PM (11 years ago)

Author:

Darin Adler

Message:

Move hasNonEmptyBoundingBox from Node to HTMLAnchorElement
​https://bugs.webkit.org/show_bug.cgi?id=116842

Reviewed by Antti Koivisto.

• dom/Node.cpp: Removed hasNonEmptyBoundingBox.
• dom/Node.h: Ditto.

• html/HTMLAnchorElement.cpp:

(WebCore::HTMLAnchorElement::isMouseFocusable): Fixed #if to be more readable and
added more-explicit clearer comments. No behavior change.
(WebCore::hasNonEmptyBox): Added. Has the same code that was in
Node::hasNonEmptyBoundingBox with some additional comments.
(WebCore::HTMLAnchorElement::isKeyboardFocusable): Call hasNonEmptyBox.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Node.cpp (modified) (1 diff)
• dom/Node.h (modified) (1 diff)
• html/HTMLAnchorElement.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-05-27  Darin Adler  <darin@apple.com>
+
+        Move hasNonEmptyBoundingBox from Node to HTMLAnchorElement
+        https://bugs.webkit.org/show_bug.cgi?id=116842
+
+        Reviewed by Antti Koivisto.
+
+        * dom/Node.cpp: Removed hasNonEmptyBoundingBox.
+        * dom/Node.h: Ditto.
+
+        * html/HTMLAnchorElement.cpp:
+        (WebCore::HTMLAnchorElement::isMouseFocusable): Fixed #if to be more readable and
+        added more-explicit clearer comments. No behavior change.
+        (WebCore::hasNonEmptyBox): Added.  Has the same code that was in
+        Node::hasNonEmptyBoundingBox with some additional comments.
+        (WebCore::HTMLAnchorElement::isKeyboardFocusable): Call hasNonEmptyBox.
+
 2013-05-27  Darin Adler  <darin@apple.com>
 

trunk/Source/WebCore/dom/Node.cpp

@@ -774 +774 @@
 }
 
-bool Node::hasNonEmptyBoundingBox() const
-{
-    // Before calling absoluteRects, check for the common case where the renderer
-    // is non-empty, since this is a faster check and almost always returns true.
-    RenderBoxModelObject* box = renderBoxModelObject();
-    if (!box)
-        return false;
-    if (!box->borderBoundingBox().isEmpty())
-        return true;
-
-    Vector<IntRect> rects;
-    FloatPoint absPos = renderer()->localToAbsolute();
-    renderer()->absoluteRects(rects, flooredLayoutPoint(absPos));
-    size_t n = rects.size();
-    for (size_t i = 0; i < n; ++i)
-        if (!rects[i].isEmpty())
-            return true;
-
-    return false;
-}
-
 inline void Node::setStyleChange(StyleChangeType changeType)
 {

trunk/Source/WebCore/dom/Node.h

@@ -427 +427 @@
     IntRect pixelSnappedRenderRect(bool* isReplaced) { return pixelSnappedIntRect(renderRect(isReplaced)); }
 
-    // Returns true if the node has a non-empty bounding box in layout.
-    // This does not 100% guarantee the user can see it, but is pretty close.
-    // Note: This method only works properly after layout has occurred.
-    bool hasNonEmptyBoundingBox() const;
-
     unsigned nodeIndex() const;
 

trunk/Source/WebCore/html/HTMLAnchorElement.cpp

@@ -95 +95 @@
 bool HTMLAnchorElement::isMouseFocusable() const
 {
-    // Anchor elements should be mouse focusable, https://bugs.webkit.org/show_bug.cgi?id=26856
-#if !PLATFORM(GTK) && !PLATFORM(QT) && !PLATFORM(EFL)
+#if !(PLATFORM(EFL) || PLATFORM(GTK) || PLATFORM(QT))
+    // Only allow links with tabIndex or contentEditable to be mouse focusable.
+    // This is our rule for the Mac platform; on many other platforms we focus any link you click on.
     if (isLink())
-        // Only allow links with tabIndex or contentEditable to be mouse focusable.
         return HTMLElement::supportsFocus();
 #endif
 
-    // Allow tab index etc to control focus.
     return HTMLElement::isMouseFocusable();
+}
+
+static bool hasNonEmptyBox(RenderModelBoxObject* renderer)
+{
+    if (!renderer)
+        return false;
+
+    // Before calling absoluteRects, check for the common case where borderBoundingBox
+    // is non-empty, since this is a faster check and almost always returns true.
+    // FIXME: Why do we need to call absoluteRects at all?
+    if (!renderer->borderBoundingBox().isEmpty())
+        return true;
+
+    // FIXME: Since all we are checking is whether the rects are empty, could we just
+    // pass in 0,0 for the layout point instead of calling localToAbsolute?
+    Vector<IntRect> rects;
+    renderer->absoluteRects(rects, flooredLayoutPoint(renderer->localToAbsolute()));
+    size_t size = rects.size();
+    for (size_t i = 0; i < size; ++i) {
+        if (!rects[i].isEmpty())
+            return true;
+    }
+
+    return false;
 }
 
@@ -123 +146 @@
         return true;
 
-    return hasNonEmptyBoundingBox();
+    return hasNonEmptyBox(renderBoxModelObject());
 }