Bug 820891 part 2. Make client* properties for tables work with the table wrapper box, not the table box. r=dholbert
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 10 Jul 2018 09:28:09 -0700
changeset 425631 d891c10133eac2d225b9f09115a09bccb95709bf
parent 425630 8853777480d141a8e988748dc47b2aac5e4c2823
child 425632 0f9af414e717ed0a9ff6bee79b098a2d6febaa54
push id105119
push userbzbarsky@mozilla.com
push dateTue, 10 Jul 2018 16:28:29 +0000
treeherdermozilla-inbound@0f9af414e717 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs820891
milestone63.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 820891 part 2. Make client* properties for tables work with the table wrapper box, not the table box. r=dholbert
dom/base/Element.cpp
dom/base/Element.h
dom/html/test/test_bug375003-1.html
dom/html/test/test_bug375003-2.html
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/css/cssom-view/table-client-props.html
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -661,34 +661,29 @@ Element::GetElementsByTagName(const nsAS
 nsIFrame*
 Element::GetStyledFrame()
 {
   nsIFrame *frame = GetPrimaryFrame(FlushType::Layout);
   return frame ? nsLayoutUtils::GetStyleFrame(frame) : nullptr;
 }
 
 nsIScrollableFrame*
-Element::GetScrollFrame(nsIFrame **aStyledFrame, FlushType aFlushType)
+Element::GetScrollFrame(nsIFrame **aFrame, FlushType aFlushType)
 {
   // it isn't clear what to return for SVG nodes, so just return nothing
   if (IsSVGElement()) {
-    if (aStyledFrame) {
-      *aStyledFrame = nullptr;
+    if (aFrame) {
+      *aFrame = nullptr;
     }
     return nullptr;
   }
 
-  // Inline version of GetStyledFrame to use the given FlushType.
   nsIFrame* frame = GetPrimaryFrame(aFlushType);
-  if (frame) {
-    frame = nsLayoutUtils::GetStyleFrame(frame);
-  }
-
-  if (aStyledFrame) {
-    *aStyledFrame = frame;
+  if (aFrame) {
+    *aFrame = frame;
   }
   if (frame) {
     // 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();
@@ -702,23 +697,18 @@ Element::GetScrollFrame(nsIFrame **aStyl
   }
 
   nsIDocument* doc = OwnerDoc();
   // Note: This IsScrollingElement() call can flush frames, if we're the body of
   // a quirks mode document.
   bool isScrollingElement = OwnerDoc()->IsScrollingElement(this);
   // Now reget *aStyledFrame if the caller asked for it, because that frame
   // flush can kill it.
-  if (aStyledFrame) {
-    nsIFrame* frame = GetPrimaryFrame(FlushType::None);
-    if (frame) {
-      *aStyledFrame = nsLayoutUtils::GetStyleFrame(frame);
-    } else {
-      *aStyledFrame = nullptr;
-    }
+  if (aFrame) {
+    *aFrame = GetPrimaryFrame(FlushType::None);
   }
 
   if (isScrollingElement) {
     // Our scroll info should map to the root scrollable frame if there is one.
     if (nsIPresShell* shell = doc->GetShell()) {
       return shell->GetRootScrollFrameAsScrollable();
     }
   }
@@ -1040,29 +1030,32 @@ Element::ScrollWidth()
   }
 
   return nsPresContext::AppUnitsToIntCSSPixels(width);
 }
 
 nsRect
 Element::GetClientAreaRect()
 {
-  nsIFrame* styledFrame;
-  nsIScrollableFrame* sf = GetScrollFrame(&styledFrame);
+  nsIFrame* frame;
+  nsIScrollableFrame* sf = GetScrollFrame(&frame);
 
   if (sf) {
     return sf->GetScrollPortRect();
   }
 
-  if (styledFrame &&
-      (styledFrame->StyleDisplay()->mDisplay != StyleDisplay::Inline ||
-       styledFrame->IsFrameOfType(nsIFrame::eReplaced))) {
+  if (frame &&
+      // The display check is OK even though we're not looking at the style
+      // frame, because the style frame only differs from "frame" for tables,
+      // and table wrappers have the same display as the table itself.
+      (frame->StyleDisplay()->mDisplay != StyleDisplay::Inline ||
+       frame->IsFrameOfType(nsIFrame::eReplaced))) {
     // Special case code to make client area work even when there isn't
     // a scroll view, see bug 180552, bug 227567.
-    return styledFrame->GetPaddingRect() - styledFrame->GetPositionIgnoringScrolling();
+    return frame->GetPaddingRect() - frame->GetPositionIgnoringScrolling();
   }
 
   // SVG nodes reach here and just return 0
   return nsRect(0, 0, 0, 0);
 }
 
 already_AddRefed<DOMRect>
 Element::GetBoundingClientRect()
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1980,18 +1980,23 @@ private:
   const nsAttrValue* GetSVGAnimatedClass() const;
 
   /**
    * Get this element's client area rect in app units.
    * @return the frame's client area
    */
   MOZ_CAN_RUN_SCRIPT nsRect GetClientAreaRect();
 
+  /**
+   * Get a scrollframe for the element, if any.  If aFrame is not null, the
+   * element's primary frame (after whatever flushing is needed) will be
+   * returned in *aFrame.
+   */
   MOZ_CAN_RUN_SCRIPT
-  nsIScrollableFrame* GetScrollFrame(nsIFrame **aStyledFrame = nullptr,
+  nsIScrollableFrame* GetScrollFrame(nsIFrame **aFrame = nullptr,
                                      FlushType aFlushType = FlushType::Layout);
 
   // Prevent people from doing pointless checks/casts on Element instances.
   void IsElement() = delete;
   void AsElement() = delete;
 
   // Data members
   EventStates mState;
--- a/dom/html/test/test_bug375003-1.html
+++ b/dom/html/test/test_bug375003-1.html
@@ -39,17 +39,17 @@ function t3(id,c,o,s,pid) {
   is(p.id, pid, id+".offsetParent");
 }
 
 function run_test() {
    t3('span1',[0,0,20,20],[12,12,20,20],[0,0,20,20],'td1');
    t3('td1'  ,[1,1,69,44],[16,16,71,46],[0,0,69,46],'table1');
    t3('tr1'  ,[0,0,71,46],[16,16,71,46],[0,0,71,44],'table1');
    t3('span2',[10,0,20,20],[27,12,30,20],[0,0,20,20],'td2');
-   t3('table1',[9,9,85,113],[10,10,103,131],[0,0,85,50],'body');
+   t3('table1',[0,0,103,131],[10,10,103,131],[0,0,85,50],'body');
    t3('div1',[10,10,-1,131],[0,0,-1,151],[0,0,-1,85],'body');
 
    t3('span2b',[10,0,20,20],[25,-1,30,20],[0,0,20,20],'body');
    // XXX not sure how to make reliable cross-platform tests for replaced-inline, inline
    // t3('span2c',[10,2,18,2],[25,-1,30,6],[0,0,30,20],'body');
    // t3('span2d',[0,0,0,0],[25,-1,10,19],[0,0,10,20],'body');
 
    t3('span3' ,[0,0,20,20],[15,0,20,20],[0,0,20,20],'td3');
@@ -65,24 +65,24 @@ function run_test() {
    t3('span6' ,[0,0,20,20],[20,58,20,20],[0,0,20,20],'div5');
    t3('table5',[0,0,40,78],[0,0,40,78],[0,0,40,78],'div5');
    t3('div5',[10,10,-1,78],[0,211,-1,98],[0,0,-1,70],'body');
 
    t3('span7' ,[0,0,20,20],[1,1,20,20],[0,0,20,20],'td7');
    t3('td7'   ,[1,1,37,22],[9,9,39,24],[0,0,37,22],'table7');
    t3('tr7'  ,[0,0,39,24],[9,9,39,24],[0,0,39,22],'table7');
    t3('span8' ,[0,0,20,20],[26,37,20,20],[0,0,20,20],'table7');
-   t3('table7',[7,7,43,54],[10,319,57,68],[0,0,43,50],'body');
+   t3('table7',[0,0,57,68],[10,319,57,68],[0,0,43,50],'body');
    t3('div7',[10,10,-1,68],[0,309,-1,88],[0,0,-1,70],'body');
 
    t3('span9' ,[0,0,20,20],[1,1,20,20],[0,0,20,20],'td9');
    t3('td9'   ,[1,1,22,22],[15,15,24,24],[0,0,22,24],'table9');
    t3('tr9'  ,[0,0,24,24],[15,15,24,24],[0,0,24,22],'table9');
    t3('span10' ,[0,0,20,20],[17,43,20,20],[0,0,20,20],'table9');
-   t3('table9',[13,13,28,34],[10,407,54,60],[0,0,28,50],'body');
+   t3('table9',[0,0,54,60],[10,407,54,60],[0,0,28,50],'body');
    t3('div9',[10,10,-1,0],[0,397,-1,20],[0,0,-1,70],'body');
 
    t3('span11' ,[0,0,20,20],[1,1,20,20],[0,0,20,20],'td11');
    t3('td11'   ,[0,0,22,22],[2,2,22,22],[0,0,22,22],'table11');
    t3('tr11'  ,[0,0,22,22],[2,2,22,22],[0,0,22,22],'table11');
    t3('span12' ,[0,0,20,20],[28,454,20,20],[0,0,20,20],'body');
    t3('table11',[0,0,26,30],[10,427,26,30],[0,0,26,50],'body');
    t3('div11',[10,10,-1,30],[0,417,-1,50],[0,0,-1,70],'body');
--- a/dom/html/test/test_bug375003-2.html
+++ b/dom/html/test/test_bug375003-2.html
@@ -63,19 +63,19 @@ function t3(id,c,o,s,pid) {
   is(p.id, pid, id+".offsetParent");
 }
 
 function run_test() {
    // XXX how test clientWidth/clientHeight (the -1 below) in cross-platform manner
    // without hard-coding the scrollbar width?
    t3('div1',[20,20,-1,-1],[10,10,200,160],[0,0,230,20],'body');
    t3('abs1',[20,20,-1,-1],[500,170,200,160],[0,0,230,20],'body');
-   t3('table1',[20,20,266,266],[10,170,306,306],[0,0,266,20],'body');
+   t3('table1',[0,0,306,306],[10,170,306,306],[0,0,266,20],'body');
    t3('table2',[0,0,206,206],[0,0,206,206],[0,0,206,20],'table2parent');
-   t3('table3',[10,10,208,208],[0,0,228,228],[0,0,208,228],'table3parent');
+   t3('table3',[0,0,228,228],[0,0,228,228],[0,0,208,228],'table3parent');
    t3('table3parent',[0,0,228,228],[300,100,228,228],[0,0,228,228],'body');
 }
 </script>
 
 </head>
 <body id="body">
 <div id="content">
 <div id="div1parent">
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -324855,16 +324855,22 @@
     ]
    ],
    "css/cssom-view/scrollintoview.html": [
     [
      "/css/cssom-view/scrollintoview.html",
      {}
     ]
    ],
+   "css/cssom-view/table-client-props.html": [
+    [
+     "/css/cssom-view/table-client-props.html",
+     {}
+    ]
+   ],
    "css/cssom-view/table-offset-props.html": [
     [
      "/css/cssom-view/table-offset-props.html",
      {}
     ]
    ],
    "css/cssom-view/ttwf-js-cssomview-getclientrects-length.html": [
     [
@@ -548798,16 +548804,20 @@
   "css/cssom-view/support/test-tl.png": [
    "956e5156fd8c0e75b1c0f3b8b3b900b653663f74",
    "support"
   ],
   "css/cssom-view/support/test-tr.png": [
    "078e1dd6dd61d36cec239ed75d02051f61fe60a5",
    "support"
   ],
+  "css/cssom-view/table-client-props.html": [
+   "54115121d05823e9317f68de5fdad4a03b94bd19",
+   "testharness"
+  ],
   "css/cssom-view/table-offset-props.html": [
    "7327b44c0f8ed0c8ff2d4a36b89255eca85a064f",
    "testharness"
   ],
   "css/cssom-view/ttwf-js-cssomview-getclientrects-length.html": [
    "7f3440e65abbe692e3c28f1f1d04671054ecc815",
    "testharness"
   ],
copy from testing/web-platform/tests/css/cssom-view/table-offset-props.html
copy to testing/web-platform/tests/css/cssom-view/table-client-props.html
--- a/testing/web-platform/tests/css/cssom-view/table-offset-props.html
+++ b/testing/web-platform/tests/css/cssom-view/table-client-props.html
@@ -1,12 +1,12 @@
 <!doctype html>
 <meta charset=utf-8>
-<title>offset* properties on tables</title>
-<link rel="help" href="https://drafts.csswg.org/cssom-view/#extensions-to-the-htmlelement-interface">
+<title>client* properties on tables</title>
+<link rel="help" href="https://drafts.csswg.org/cssom-view/#extension-to-the-element-interface">
 <script src=/resources/testharness.js></script>
 <script src=/resources/testharnessreport.js></script>
 <div id="test-target" style="position: absolute"></div>
 <script>
   test(function() {
     // Each test consists of four elements: the markup to use, the expected
     // value of offsetWidth on the table, the expected value of offsetHeight
     // on the table, and the description string.
@@ -68,16 +68,16 @@
 
     function table() {
       return target().querySelector("table");
     }
 
     for (var t of tests) {
       test(function() {
         target().innerHTML = t[0];
-        assert_equals(table().offsetWidth, t[1], t[3] + " offsetWidth");
-        assert_equals(table().offsetHeight, t[2], t[3] + " offsetHeight");
-        assert_equals(table().offsetLeft, 0, t[3] + " offsetLeft");
-        assert_equals(table().offsetTop, 0, t[3] + " offsetTop");
+        assert_equals(table().clientWidth, t[1], t[3] + " clientWidth");
+        assert_equals(table().clientHeight, t[2], t[3] + " clientHeight");
+        assert_equals(table().clientLeft, 0, t[3] + " clientLeft");
+        assert_equals(table().clientTop, 0, t[3] + " clientTop");
       }, t[3]);
     }
   }, "Overall test to make sure there are no exceptions");
 </script>