Changeset 220313 in webkit

Timestamp:

Aug 5, 2017 1:08:16 AM (7 years ago)

Author:

Carlos Garcia Campos

Message:

getClientRects doesn't work with list box option elements
​https://bugs.webkit.org/show_bug.cgi?id=175016

Reviewed by Darin Adler.

Source/WebCore:

Since HTMLOptionElement and HTMLOptGroupElement don't have a renderer, we are always returning an empty list
from getClientRects. This is working fine in both chromium and firefox, option elements return its own bounding
box and group elements return the bounding box of the group label and all its children items.

Test: fast/dom/HTMLSelectElement/listbox-items-client-rects.html

• dom/Element.cpp:

(WebCore::listBoxElementBoundingBox): Helper function to return the bounding box of a HTMLOptionElement or
HTMLOptGroupElement element.
(WebCore::Element::getClientRects): Use listBoxElementBoundingBox() in case of HTMLOptionElement or
HTMLOptGroupElement.
(WebCore::Element::boundingClientRect): Ditto.

LayoutTests:

Add new test to check list box option elements client rects.

• fast/dom/HTMLSelectElement/listbox-items-client-rects-expected.txt: Added.
• fast/dom/HTMLSelectElement/listbox-items-client-rects.html: Added.
• platform/ios-simulator-wk2/fast/dom/HTMLSelectElement/listbox-items-client-rects-expected.txt: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/HTMLSelectElement/listbox-items-client-rects-expected.txt (added)
• LayoutTests/fast/dom/HTMLSelectElement/listbox-items-client-rects.html (added)
• LayoutTests/platform/ios-simulator-wk2/fast (added)
• LayoutTests/platform/ios-simulator-wk2/fast/dom (added)
• LayoutTests/platform/ios-simulator-wk2/fast/dom/HTMLSelectElement (added)
• LayoutTests/platform/ios-simulator-wk2/fast/dom/HTMLSelectElement/listbox-items-client-rects-expected.txt (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Element.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-08-05  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        getClientRects doesn't work with list box option elements
+        https://bugs.webkit.org/show_bug.cgi?id=175016
+
+        Reviewed by Darin Adler.
+
+        Add new test to check list box option elements client rects.
+
+        * fast/dom/HTMLSelectElement/listbox-items-client-rects-expected.txt: Added.
+        * fast/dom/HTMLSelectElement/listbox-items-client-rects.html: Added.
+        * platform/ios-simulator-wk2/fast/dom/HTMLSelectElement/listbox-items-client-rects-expected.txt: Added.
+
 2017-08-04  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-08-05  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        getClientRects doesn't work with list box option elements
+        https://bugs.webkit.org/show_bug.cgi?id=175016
+
+        Reviewed by Darin Adler.
+
+        Since HTMLOptionElement and HTMLOptGroupElement don't have a renderer, we are always returning an empty list
+        from getClientRects. This is working fine in both chromium and firefox, option elements return its own bounding
+        box and group elements return the bounding box of the group label and all its children items.
+
+        Test: fast/dom/HTMLSelectElement/listbox-items-client-rects.html
+
+        * dom/Element.cpp:
+        (WebCore::listBoxElementBoundingBox): Helper function to return the bounding box of a HTMLOptionElement or
+        HTMLOptGroupElement element.
+        (WebCore::Element::getClientRects): Use listBoxElementBoundingBox() in case of HTMLOptionElement or
+        HTMLOptGroupElement.
+        (WebCore::Element::boundingClientRect): Ditto.
+
 2017-08-04  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -62 +62 @@
 #include "HTMLNameCollection.h"
 #include "HTMLObjectElement.h"
+#include "HTMLOptGroupElement.h"
+#include "HTMLOptionElement.h"
 #include "HTMLParserIdioms.h"
+#include "HTMLSelectElement.h"
 #include "HTMLTemplateElement.h"
 #include "IdChangeInvalidation.h"
@@ -79 +82 @@
 #include "RenderFlowThread.h"
 #include "RenderLayer.h"
+#include "RenderListBox.h"
 #include "RenderNamedFlowFragment.h"
 #include "RenderRegion.h"
@@ -1146 +1150 @@
 }
 
+static std::optional<std::pair<RenderObject*, LayoutRect>> listBoxElementBoundingBox(Element& element)
+{
+    HTMLSelectElement* selectElement;
+    bool isGroup;
+    if (is<HTMLOptionElement>(element)) {
+        selectElement = downcast<HTMLOptionElement>(element).ownerSelectElement();
+        isGroup = false;
+    } else if (is<HTMLOptGroupElement>(element)) {
+        selectElement = downcast<HTMLOptGroupElement>(element).ownerSelectElement();
+        isGroup = true;
+    } else
+        return std::nullopt;
+
+    if (!selectElement || !selectElement->renderer() || !is<RenderListBox>(selectElement->renderer()))
+        return std::nullopt;
+
+    auto& renderer = downcast<RenderListBox>(*selectElement->renderer());
+    std::optional<LayoutRect> boundingBox;
+    int optionIndex = 0;
+    for (auto* item : selectElement->listItems()) {
+        if (item == &element) {
+            LayoutPoint additionOffset;
+            boundingBox = renderer.itemBoundingBoxRect(additionOffset, optionIndex);
+            if (!isGroup)
+                break;
+        } else if (isGroup && boundingBox) {
+            if (item->parentNode() != &element)
+                break;
+            LayoutPoint additionOffset;
+            boundingBox->setHeight(boundingBox->height() + renderer.itemBoundingBoxRect(additionOffset, optionIndex).height());
+        }
+        ++optionIndex;
+    }
+
+    if (!boundingBox)
+        return std::nullopt;
+
+    return std::pair<RenderObject*, LayoutRect> { &renderer, boundingBox.value() };
+}
+
 Ref<DOMRectList> Element::getClientRects()
 {
     document().updateLayoutIgnorePendingStylesheets();
 
-    RenderBoxModelObject* renderBoxModelObject = this->renderBoxModelObject();
-    if (!renderBoxModelObject)
-        return DOMRectList::create();
+    RenderObject* renderer = this->renderer();
+    Vector<FloatQuad> quads;
+
+    if (auto pair = listBoxElementBoundingBox(*this)) {
+        renderer = pair.value().first;
+        quads.append(renderer->localToAbsoluteQuad(FloatQuad { pair.value().second }));
+    } else if (auto* renderBoxModelObject = this->renderBoxModelObject())
+        renderBoxModelObject->absoluteQuads(quads);
 
     // FIXME: Handle SVG elements.
     // FIXME: Handle table/inline-table with a caption.
 
+    if (quads.isEmpty())
+        return DOMRectList::create();
+
+    document().convertAbsoluteToClientQuads(quads, renderer->style());
+    return DOMRectList::create(quads);
+}
+
+FloatRect Element::boundingClientRect()
+{
+    document().updateLayoutIgnorePendingStylesheets();
+
+    RenderObject* renderer = this->renderer();
     Vector<FloatQuad> quads;
-    renderBoxModelObject->absoluteQuads(quads);
-    document().convertAbsoluteToClientQuads(quads, renderBoxModelObject->style());
-    return DOMRectList::create(quads);
-}
-
-FloatRect Element::boundingClientRect()
-{
-    document().updateLayoutIgnorePendingStylesheets();
-
-    Vector<FloatQuad> quads;
-    if (isSVGElement() && renderer() && !renderer()->isSVGRoot()) {
+    if (isSVGElement() && renderer && !renderer->isSVGRoot()) {
         // Get the bounding rectangle from the SVG model.
         SVGElement& svgElement = downcast<SVGElement>(*this);
         FloatRect localRect;
         if (svgElement.getBoundingBox(localRect))
-            quads.append(renderer()->localToAbsoluteQuad(localRect));
-    } else {
-        // Get the bounding rectangle from the box model.
-        if (renderBoxModelObject())
-            renderBoxModelObject()->absoluteQuads(quads);
-    }
+            quads.append(renderer->localToAbsoluteQuad(localRect));
+    } else if (auto pair = listBoxElementBoundingBox(*this)) {
+        renderer = pair.value().first;
+        quads.append(renderer->localToAbsoluteQuad(FloatQuad { pair.value().second }));
+    } else if (auto* renderBoxModelObject = this->renderBoxModelObject())
+        renderBoxModelObject->absoluteQuads(quads);
 
     if (quads.isEmpty())
@@ -1187 +1238 @@
         result.unite(quads[i].boundingBox());
 
-    document().convertAbsoluteToClientRect(result, renderer()->style());
+    document().convertAbsoluteToClientRect(result, renderer->style());
     return result;
 }