Bug 1389274 - Correct the behavior of Element.scrollIntoView to match the draft spec and pass web platform tests; r=bkelly,annevk draft
authorThomas Wisniewski <wisniewskit@gmail.com>
Thu, 28 Sep 2017 16:57:24 -0400
changeset 672178 665b7207443b5be8d0e74b9839c836be3e162cda
parent 672102 3177c1b64ffe7ba5c08851791bd495421dcedb05
child 733739 cdf360fe3ce41f2bb88c53522bd5dfbf4f86c301
push id82178
push userwisniewskit@gmail.com
push dateThu, 28 Sep 2017 21:02:33 +0000
reviewersbkelly, annevk
bugs1389274
milestone58.0a1
Bug 1389274 - Correct the behavior of Element.scrollIntoView to match the draft spec and pass web platform tests; r=bkelly,annevk MozReview-Commit-ID: 3is36wstsdb
dom/base/Element.cpp
dom/base/Element.h
dom/webidl/Element.webidl
testing/web-platform/meta/MANIFEST.json
testing/web-platform/meta/cssom-view/scrollIntoView-empty-args.html.ini
testing/web-platform/meta/cssom-view/scrollIntoView-shadow.html.ini
testing/web-platform/meta/cssom-view/scrollIntoView-smooth.html.ini
testing/web-platform/meta/cssom-view/scrollintoview.html.ini
testing/web-platform/tests/cssom-view/scrollIntoView-empty-args.html
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -723,61 +723,99 @@ Element::GetScrollFrame(nsIFrame **aStyl
       return shell->GetRootScrollFrameAsScrollable();
     }
   }
 
   return nullptr;
 }
 
 void
-Element::ScrollIntoView()
+Element::ScrollIntoView(const BooleanOrScrollIntoViewOptions& aObject)
 {
-  ScrollIntoView(ScrollIntoViewOptions());
-}
-
-void
-Element::ScrollIntoView(bool aTop)
-{
+  if (aObject.IsScrollIntoViewOptions()) {
+    return ScrollIntoView(aObject.GetAsScrollIntoViewOptions());
+  }
+
+  MOZ_DIAGNOSTIC_ASSERT(aObject.IsBoolean());
+
   ScrollIntoViewOptions options;
-  if (!aTop) {
+  if (aObject.GetAsBoolean()) {
+    options.mBlock = ScrollLogicalPosition::Start;
+    options.mInline = ScrollLogicalPosition::Nearest;
+  } else {
     options.mBlock = ScrollLogicalPosition::End;
-  }
-  ScrollIntoView(options);
+    options.mInline = ScrollLogicalPosition::Nearest;
+  }
+  return ScrollIntoView(options);
 }
 
 void
 Element::ScrollIntoView(const ScrollIntoViewOptions &aOptions)
 {
   nsIDocument *document = GetComposedDoc();
   if (!document) {
     return;
   }
 
   // Get the presentation shell
   nsCOMPtr<nsIPresShell> presShell = document->GetShell();
   if (!presShell) {
     return;
   }
 
-  int16_t vpercent = (aOptions.mBlock == ScrollLogicalPosition::Start)
-                       ? nsIPresShell::SCROLL_TOP
-                       : nsIPresShell::SCROLL_BOTTOM;
+  int16_t vpercent = nsIPresShell::SCROLL_CENTER;
+  switch (aOptions.mBlock) {
+    case ScrollLogicalPosition::Start:
+      vpercent = nsIPresShell::SCROLL_TOP;
+      break;
+    case ScrollLogicalPosition::Center:
+      vpercent = nsIPresShell::SCROLL_CENTER;
+      break;
+    case ScrollLogicalPosition::End:
+      vpercent = nsIPresShell::SCROLL_BOTTOM;
+      break;
+    case ScrollLogicalPosition::Nearest:
+      vpercent = nsIPresShell::SCROLL_MINIMUM;
+      break;
+    default:
+      MOZ_ASSERT_UNREACHABLE("Unexpected ScrollLogicalPosition value");
+  }
+
+  int16_t hpercent = nsIPresShell::SCROLL_CENTER;
+  switch (aOptions.mInline) {
+    case ScrollLogicalPosition::Start:
+      hpercent = nsIPresShell::SCROLL_LEFT;
+      break;
+    case ScrollLogicalPosition::Center:
+      hpercent = nsIPresShell::SCROLL_CENTER;
+      break;
+    case ScrollLogicalPosition::End:
+      hpercent = nsIPresShell::SCROLL_RIGHT;
+      break;
+    case ScrollLogicalPosition::Nearest:
+      hpercent = nsIPresShell::SCROLL_MINIMUM;
+      break;
+    default:
+      MOZ_ASSERT_UNREACHABLE("Unexpected ScrollLogicalPosition value");
+  }
 
   uint32_t flags = nsIPresShell::SCROLL_OVERFLOW_HIDDEN;
   if (aOptions.mBehavior == ScrollBehavior::Smooth) {
     flags |= nsIPresShell::SCROLL_SMOOTH;
   } else if (aOptions.mBehavior == ScrollBehavior::Auto) {
     flags |= nsIPresShell::SCROLL_SMOOTH_AUTO;
   }
 
   presShell->ScrollContentIntoView(this,
                                    nsIPresShell::ScrollAxis(
                                      vpercent,
                                      nsIPresShell::SCROLL_ALWAYS),
-                                   nsIPresShell::ScrollAxis(),
+                                   nsIPresShell::ScrollAxis(
+                                     hpercent,
+                                     nsIPresShell::SCROLL_ALWAYS),
                                    flags);
 }
 
 void
 Element::Scroll(const CSSIntPoint& aScroll, const ScrollOptions& aOptions)
 {
   nsIScrollableFrame* sf = GetScrollFrame();
   if (sf) {
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1074,19 +1074,20 @@ public:
   already_AddRefed<DestinationInsertionPointList> GetDestinationInsertionPoints();
 
   ShadowRoot *FastGetShadowRoot() const
   {
     nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots();
     return slots ? slots->mShadowRoot.get() : nullptr;
   }
 
-  void ScrollIntoView();
-  void ScrollIntoView(bool aTop);
+private:
   void ScrollIntoView(const ScrollIntoViewOptions &aOptions);
+public:
+  void ScrollIntoView(const BooleanOrScrollIntoViewOptions& aObject);
   void Scroll(double aXScroll, double aYScroll);
   void Scroll(const ScrollToOptions& aOptions);
   void ScrollTo(double aXScroll, double aYScroll);
   void ScrollTo(const ScrollToOptions& aOptions);
   void ScrollBy(double aXScrollDif, double aYScrollDif);
   void ScrollBy(const ScrollToOptions& aOptions);
   /* Scrolls without flushing the layout.
    * aDx is the x offset, aDy the y offset in CSS pixels.
--- a/dom/webidl/Element.webidl
+++ b/dom/webidl/Element.webidl
@@ -166,29 +166,29 @@ interface Element : Node {
   DOMMatrixReadOnly getTransformToAncestor(Element ancestor);
   [ChromeOnly]
   DOMMatrixReadOnly getTransformToParent();
   [ChromeOnly]
   DOMMatrixReadOnly getTransformToViewport();
 };
 
 // http://dev.w3.org/csswg/cssom-view/
-enum ScrollLogicalPosition { "start", "end" };
+enum ScrollLogicalPosition { "start", "center", "end", "nearest" };
 dictionary ScrollIntoViewOptions : ScrollOptions {
   ScrollLogicalPosition block = "start";
+  ScrollLogicalPosition inline = "nearest";
 };
 
 // http://dev.w3.org/csswg/cssom-view/#extensions-to-the-element-interface
 partial interface Element {
   DOMRectList getClientRects();
   DOMRect getBoundingClientRect();
 
   // scrolling
-  void scrollIntoView(boolean top);
-  void scrollIntoView(optional ScrollIntoViewOptions options);
+  void scrollIntoView(optional (boolean or ScrollIntoViewOptions) arg);
   // None of the CSSOM attributes are [Pure], because they flush
            attribute long scrollTop;   // scroll on setting
            attribute long scrollLeft;  // scroll on setting
   readonly attribute long scrollWidth;
   readonly attribute long scrollHeight;
   
   void scroll(unrestricted double x, unrestricted double y);
   void scroll(optional ScrollToOptions options);
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -337734,22 +337734,16 @@
     ]
    ],
    "cssom-view/offsetTopLeftInScrollableParent.html": [
     [
      "/cssom-view/offsetTopLeftInScrollableParent.html",
      {}
     ]
    ],
-   "cssom-view/scrollIntoView-empty-args.html": [
-    [
-     "/cssom-view/scrollIntoView-empty-args.html",
-     {}
-    ]
-   ],
    "cssom-view/scrollIntoView-shadow.html": [
     [
      "/cssom-view/scrollIntoView-shadow.html",
      {}
     ]
    ],
    "cssom-view/scrollIntoView-smooth.html": [
     [
@@ -578699,20 +578693,16 @@
   "cssom-view/resources/iframe2.html": [
    "0a8784c474ccdd4a3e76cb936855a8ef59566217",
    "support"
   ],
   "cssom-view/scrollBoundaryBehavior-manual.html": [
    "987051cdbad355cbb1bbb8ea1030a3b17e533f09",
    "manual"
   ],
-  "cssom-view/scrollIntoView-empty-args.html": [
-   "57e22136750f54145c37722674389590b7f340b6",
-   "testharness"
-  ],
   "cssom-view/scrollIntoView-shadow.html": [
    "3c4a18992105fd7bf19cbf29f0b6d80cb12ca98c",
    "testharness"
   ],
   "cssom-view/scrollIntoView-smooth.html": [
    "0561564f185dcaf2ad3a8e14e081efb3c2c273e3",
    "testharness"
   ],
deleted file mode 100644
--- a/testing/web-platform/meta/cssom-view/scrollIntoView-empty-args.html.ini
+++ /dev/null
@@ -1,11 +0,0 @@
-[scrollIntoView-empty-args.html]
-  type: testharness
-  [scrollIntoView should behave correctly when the arg is not fully specified as ScrollIntoViewOptions]
-    expected: FAIL
-
-  [scrollIntoView should behave correctly when the arg is [object Object\]]
-    expected: FAIL
-
-  [scrollIntoView should behave correctly when the arg is null]
-    expected: FAIL
-
--- a/testing/web-platform/meta/cssom-view/scrollIntoView-shadow.html.ini
+++ b/testing/web-platform/meta/cssom-view/scrollIntoView-shadow.html.ini
@@ -1,5 +1,8 @@
 [scrollIntoView-shadow.html]
   type: testharness
   [scrollIntoView should behave correctly if applies to shadow dom elements]
-    expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1293844
+    expected:
+      if stylo: FAIL
+      PASS
 
deleted file mode 100644
--- a/testing/web-platform/meta/cssom-view/scrollIntoView-smooth.html.ini
+++ /dev/null
@@ -1,11 +0,0 @@
-[scrollIntoView-smooth.html]
-  type: testharness
-  [Smooth scrollIntoView should scroll the element to the 'nearest' position]
-    expected: FAIL
-
-  [Smooth scrollIntoView should scroll the element to the 'start' position]
-    expected: FAIL
-
-  [Smooth scrollIntoView should scroll the element to the 'center' position]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/cssom-view/scrollintoview.html.ini
+++ /dev/null
@@ -1,38 +0,0 @@
-[scrollintoview.html]
-  type: testharness
-  [scrollIntoView({block: "center", inline: "center"}) starting at left,top]
-    expected: FAIL
-
-  [scrollIntoView({block: "center", inline: "center"}) starting at left,bottom]
-    expected: FAIL
-
-  [scrollIntoView({block: "center", inline: "center"}) starting at right,top]
-    expected: FAIL
-
-  [scrollIntoView({block: "center", inline: "center"}) starting at right,bottom]
-    expected: FAIL
-
-  [scrollIntoView({block: "start", inline: "start"}) starting at left,top]
-    expected: FAIL
-
-  [scrollIntoView({block: "start", inline: "start"}) starting at left,bottom]
-    expected: FAIL
-
-  [scrollIntoView({block: "end", inline: "end"}) starting at right,top]
-    expected: FAIL
-
-  [scrollIntoView({block: "end", inline: "end"}) starting at right,bottom]
-    expected: FAIL
-
-  [scrollIntoView({block: "nearest", inline: "nearest"}) starting at left,top]
-    expected: FAIL
-
-  [scrollIntoView({block: "nearest", inline: "nearest"}) starting at left,bottom]
-    expected: FAIL
-
-  [scrollIntoView({block: "nearest", inline: "nearest"}) starting at right,top]
-    expected: FAIL
-
-  [scrollIntoView({block: "nearest", inline: "nearest"}) starting at right,bottom]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/tests/cssom-view/scrollIntoView-empty-args.html
+++ /dev/null
@@ -1,83 +0,0 @@
-<!DOCTYPE HTML>
-<script src="/resources/testharness.js"></script>
-<script src="/resources/testharnessreport.js"></script>
-<title>Check End Position of scrollIntoView when arg is not fully specified</title>
-
-<style>
-  #container {
-    position: relative;
-    height: 1000px;
-    width: 800px;
-    overflow: scroll;
-  }
-
-  #content {
-    position: absolute;
-    height: 500px;
-    width: 400px;
-    left: 1000px;
-    top: 1000px;
-    background-color: red;
-  }
-</style>
-
-<div id="container">
-  <div id="filler" style="height: 2500px; width: 2500px"></div>
-  <div id="content">I must become visible</div>
-</div>
-
-<script>
-  add_completion_callback(() => document.getElementById("container").remove());
-  var content = document.getElementById("content");
-  var container = document.getElementById("container");
-
-  var remaining_width = container.clientWidth - content.clientWidth;
-  var remaining_height = container.clientHeight - content.clientHeight;
-
-  function instantScrollToTestArgs(arg, expected_x, expected_y) {
-    test(t => {
-      container.scrollTop = container.scrollLeft = 0;
-
-      assert_not_equals(container.scrollLeft, expected_x);
-      assert_not_equals(container.scrollTop, expected_y);
-      if (arg == "omitted")
-        content.scrollIntoView();
-      else
-        content.scrollIntoView(arg);
-      assert_approx_equals(container.scrollTop, expected_y, 1, "verify scroll top");
-      assert_approx_equals(container.scrollLeft, expected_x, 1, "verify scroll left");
-
-    }, "scrollIntoView should behave correctly when the arg is " + arg);
-  }
-
-  // expected alignment: inline => nearest, block => start
-  instantScrollToTestArgs("omitted",
-    content.offsetLeft - remaining_width,
-    content.offsetTop);
-
-  // expected alignment: inline => nearest, block => start
-  instantScrollToTestArgs(true,
-    content.offsetLeft - remaining_width,
-    content.offsetTop);
-
-  // expected alignment: inline => nearest, block => end
-  instantScrollToTestArgs(false,
-    content.offsetLeft - remaining_width,
-    content.offsetTop - remaining_height);
-
-  // expected alignment: inline => center, block => center
-  instantScrollToTestArgs({},
-    content.offsetLeft - remaining_width / 2,
-    content.offsetTop - remaining_height / 2);
-
-  // expected alignment: inline => center, block => center
-  instantScrollToTestArgs(null,
-    content.offsetLeft - remaining_width / 2,
-    content.offsetTop - remaining_height / 2);
-
-  // expected alignment: inline => nearest, block => start
-  instantScrollToTestArgs(undefined,
-    content.offsetLeft - remaining_width,
-    content.offsetTop);
-
-</script>
\ No newline at end of file