Bug 874811 - Outer svg is a replaced box with CSS layout, and CSSOM should reflect that accordingly. r=dholbert,violet
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 22 Apr 2019 17:01:10 +0000
changeset 470381 81021da13bce7f15ef4129ad300af8894cd2af3f
parent 470380 27c8b8f5b3c81e5f216a4ac948c7d082e19fc287
child 470382 a11bd690638f3d2cb4e5b27450f87f808eebdfef
child 470385 040c255450c993aab51f1c6ba989203e1f94efa8
push id112868
push useropoprus@mozilla.com
push dateMon, 22 Apr 2019 22:19:22 +0000
treeherdermozilla-inbound@24537856cc88 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, violet
bugs874811
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 874811 - Outer svg is a replaced box with CSS layout, and CSSOM should reflect that accordingly. r=dholbert,violet Co-authored-by: violet <violet.bugreport@gmail.com> Differential Revision: https://phabricator.services.mozilla.com/D28304
dom/base/Element.cpp
dom/tests/mochitest/general/test_offsets.js
layout/style/nsComputedDOMStyle.cpp
layout/svg/nsSVGOuterSVGFrame.h
testing/web-platform/tests/css/cssom-view/outer-svg.html
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -659,29 +659,26 @@ void Element::GetAttributeNames(nsTArray
 
 already_AddRefed<nsIHTMLCollection> Element::GetElementsByTagName(
     const nsAString& aLocalName) {
   return NS_GetContentList(this, kNameSpaceID_Unknown, aLocalName);
 }
 
 nsIScrollableFrame* Element::GetScrollFrame(nsIFrame** aFrame,
                                             FlushType aFlushType) {
-  // it isn't clear what to return for SVG nodes, so just return nothing
-  if (IsSVGElement()) {
-    if (aFrame) {
-      *aFrame = nullptr;
-    }
-    return nullptr;
-  }
-
   nsIFrame* frame = GetPrimaryFrame(aFlushType);
   if (aFrame) {
     *aFrame = frame;
   }
   if (frame) {
+    if (frame->HasAnyStateBits(NS_FRAME_SVG_LAYOUT)) {
+      // It's unclear what to return for SVG frames, so just return null.
+      return nullptr;
+    }
+
     // menu frames implement GetScrollTargetFrame but we don't want
     // to use it here.  Similar for comboboxes.
     LayoutFrameType type = frame->Type();
     if (type != LayoutFrameType::Menu &&
         type != LayoutFrameType::ComboboxControl) {
       nsIScrollableFrame* scrollFrame = frame->GetScrollTargetFrame();
       if (scrollFrame) {
         MOZ_ASSERT(!OwnerDoc()->IsScrollingElement(this),
@@ -927,17 +924,17 @@ void Element::SetScrollLeft(int32_t aScr
 void Element::MozScrollSnap() {
   nsIScrollableFrame* sf = GetScrollFrame(nullptr, FlushType::None);
   if (sf) {
     sf->ScrollSnap();
   }
 }
 
 static nsSize GetScrollRectSizeForOverflowVisibleFrame(nsIFrame* aFrame) {
-  if (!aFrame) {
+  if (!aFrame || aFrame->HasAnyStateBits(NS_FRAME_SVG_LAYOUT)) {
     return nsSize(0, 0);
   }
 
   nsRect paddingRect = aFrame->GetPaddingRectRelativeToSelf();
   nsOverflowAreas overflowAreas(paddingRect, paddingRect);
   // Add the scrollable overflow areas of children (if any) to the paddingRect.
   // It's important to start with the paddingRect, otherwise if there are no
   // children the overflow rect will be 0,0,0,0 which will force the point 0,0
@@ -949,33 +946,29 @@ static nsSize GetScrollRectSizeForOverfl
       overflowAreas.ScrollableOverflow().UnionEdges(paddingRect);
   return nsLayoutUtils::GetScrolledRect(aFrame, overflowRect,
                                         paddingRect.Size(),
                                         aFrame->StyleVisibility()->mDirection)
       .Size();
 }
 
 int32_t Element::ScrollHeight() {
-  if (IsSVGElement()) return 0;
-
   nsIFrame* frame;
   nsIScrollableFrame* sf = GetScrollFrame(&frame);
   nscoord height;
   if (sf) {
     height = sf->GetScrollRange().Height() + sf->GetScrollPortRect().Height();
   } else {
     height = GetScrollRectSizeForOverflowVisibleFrame(frame).height;
   }
 
   return nsPresContext::AppUnitsToIntCSSPixels(height);
 }
 
 int32_t Element::ScrollWidth() {
-  if (IsSVGElement()) return 0;
-
   nsIFrame* frame;
   nsIScrollableFrame* sf = GetScrollFrame(&frame);
   nscoord width;
   if (sf) {
     width = sf->GetScrollRange().Width() + sf->GetScrollPortRect().Width();
   } else {
     width = GetScrollRectSizeForOverflowVisibleFrame(frame).width;
   }
--- a/dom/tests/mochitest/general/test_offsets.js
+++ b/dom/tests/mochitest/general/test_offsets.js
@@ -27,16 +27,20 @@ function testElements(baseid, callback)
   div.scrollLeft = 10;
   div.scrollTop = 10;
   is(element.scrollLeft, 0, element.id + " scrollLeft after nonscroll");
   is(element.scrollTop, 0, element.id + " scrollTop after nonscroll");
 
   callback();
 }
 
+function usesSVGLayout(e) {
+  return e instanceof SVGElement && !(e instanceof SVGSVGElement);
+}
+
 function toNearestAppunit(v)
 {
   // 60 appunits per CSS pixel; round result to the nearest appunit
   return Math.round(v*60)/60;
 }
 
 function isEqualAppunits(a, b, msg)
 {
@@ -89,23 +93,23 @@ function testElement(element)
       doScrollCheck = false;
     } else {
       scrollWidth = clientWidth;
       scrollHeight = clientHeight;
     }
   }
 
   if (doScrollCheck) {
-    if (element instanceof SVGElement)
+    if (usesSVGLayout(element))
       checkScrollState(element, 0, 0, 0, 0, element.id);
      else
       checkScrollState(element, 0, 0, scrollWidth, scrollHeight, element.id);
   }
 
-  if (element instanceof SVGElement)
+  if (usesSVGLayout(element))
     checkClientState(element, 0, 0, 0, 0, element.id);
   else
     checkClientState(element, borderLeft, borderTop, clientWidth, clientHeight, element.id);
 
   var boundingrect = element.getBoundingClientRect();
   isEqualAppunits(boundingrect.width, borderLeft + paddingLeft + width + paddingRight + borderRight,
      element.id + " bounding rect width");
   isEqualAppunits(boundingrect.height, borderTop + paddingTop + height + paddingBottom + borderBottom,
@@ -202,27 +206,27 @@ function checkCoords(element, type, left
     // scrollWidth and scrollHeight can deviate by 1 pixel due to snapping.
     checkCoordFuzzy(element, type + "Width", width, 1, testname);
     checkCoordFuzzy(element, type + "Height", height, 1, testname);
   } else {
     checkCoord(element, type + "Width", width, testname);
     checkCoord(element, type + "Height", height, testname);
   }
 
-  if (element instanceof SVGElement)
+  if (usesSVGLayout(element))
     return;
 
   if (element.id == "outerpopup" && !element.parentNode.open) // closed popup
     return;
 
   if (element.id == "div-displaynone" || element.id == "nonappended") // hidden elements
     ok(element[type + "Width"] == 0 && element[type + "Height"] == 0,
        element.id + " has zero " + type + " width and height");
 }
 
 function gcs(element, prop)
 {
-  var propVal = (element instanceof SVGElement && (prop == "width" || prop == "height")) ?
+  var propVal = (usesSVGLayout(element) && (prop == "width" || prop == "height")) ?
                    element.getAttribute(prop) : getComputedStyle(element, "")[prop];
   if (propVal == "auto" || element.id == "nonappended")
     return 0;
   return parseFloat(propVal);
 }
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -1826,19 +1826,17 @@ already_AddRefed<CSSValue> nsComputedDOM
 
   bool calcHeight = false;
 
   if (mInnerFrame) {
     calcHeight = true;
 
     const nsStyleDisplay* displayData = StyleDisplay();
     if (displayData->mDisplay == mozilla::StyleDisplay::Inline &&
-        !(mInnerFrame->IsFrameOfType(nsIFrame::eReplaced)) &&
-        // An outer SVG frame should behave the same as eReplaced in this case
-        !mInnerFrame->IsSVGOuterSVGFrame()) {
+        !mInnerFrame->IsFrameOfType(nsIFrame::eReplaced)) {
       calcHeight = false;
     }
   }
 
   if (calcHeight) {
     AssertFlushedPendingReflows();
     nsMargin adjustedValues = GetAdjustedValuesForBoxSizing();
     val->SetAppUnits(mInnerFrame->GetContentRect().height +
@@ -1865,19 +1863,17 @@ already_AddRefed<CSSValue> nsComputedDOM
 
   bool calcWidth = false;
 
   if (mInnerFrame) {
     calcWidth = true;
 
     const nsStyleDisplay* displayData = StyleDisplay();
     if (displayData->mDisplay == mozilla::StyleDisplay::Inline &&
-        !(mInnerFrame->IsFrameOfType(nsIFrame::eReplaced)) &&
-        // An outer SVG frame should behave the same as eReplaced in this case
-        !mInnerFrame->IsSVGOuterSVGFrame()) {
+        !mInnerFrame->IsFrameOfType(nsIFrame::eReplaced)) {
       calcWidth = false;
     }
   }
 
   if (calcWidth) {
     AssertFlushedPendingReflows();
     nsMargin adjustedValues = GetAdjustedValuesForBoxSizing();
     val->SetAppUnits(mInnerFrame->GetContentRect().width +
--- a/layout/svg/nsSVGOuterSVGFrame.h
+++ b/layout/svg/nsSVGOuterSVGFrame.h
@@ -70,17 +70,18 @@ class nsSVGOuterSVGFrame final : public 
   virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                 const nsDisplayListSet& aLists) override;
 
   virtual void Init(nsIContent* aContent, nsContainerFrame* aParent,
                     nsIFrame* aPrevInFlow) override;
 
   bool IsFrameOfType(uint32_t aFlags) const override {
     return nsSVGDisplayContainerFrame::IsFrameOfType(
-        aFlags & ~eSupportsContainLayoutAndPaint);
+        aFlags &
+        ~(eSupportsContainLayoutAndPaint | eReplaced | eReplacedSizing));
   }
 
 #ifdef DEBUG_FRAME_DUMP
   virtual nsresult GetFrameName(nsAString& aResult) const override {
     return MakeFrameName(NS_LITERAL_STRING("SVGOuterSVG"), aResult);
   }
 #endif
 
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/cssom-view/outer-svg.html
@@ -0,0 +1,35 @@
+<!doctype html>
+<title>CSS Tests: client* and scroll* APIs work as expected with outer SVG elements</title>
+<link rel="help" href="https://drafts.csswg.org/cssom-view/#extension-to-the-element-interface">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=874811">
+<link rel="author" title="violet" href="mailto:violet.bugreport@gmail.com">
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+#u {
+  padding: 30px;
+  transform: translate(50px,60px) scale(2,3);
+  border: 5px solid lime;
+  width: 50px;
+  height: 100px;
+}
+</style>
+<div style="width: 100px; height: 2000px; border: 1px solid blue"></div>
+<svg id="u"></svg>
+<script>
+let u = document.getElementById("u");
+test(function() {
+  assert_equals(u.clientTop, 5, "u.clientTop");
+  assert_equals(u.clientLeft, 5, "u.clientLeft");
+  assert_equals(u.clientWidth, 110, "u.clientWidth");
+  assert_equals(u.clientHeight, 160, "u.clientHeight");
+}, "clientWidth, clientHeight, clientTop and clientLeft work on outer svg element");
+test(function() {
+  assert_equals(u.scrollTop, 0, "u.scrollTop");
+  assert_equals(u.scrollLeft, 0, "u.scrollLeft");
+  assert_equals(u.scrollWidth, 110, "u.scrollWidth");
+  assert_equals(u.scrollHeight, 160, "u.scrollHeight");
+}, "scrollWidth, scrollHeight, scrollTop and scrollLeft work on outer svg element");
+</script>