Bug 1389274 - Correct the behavior of Element.scrollIntoView to match the draft spec and pass web platform tests; r=annevk,bkelly
authorThomas Wisniewski <wisniewskit@gmail.com>
Thu, 28 Sep 2017 16:57:24 -0400
changeset 434420 2a91649c5285aaad5ef33362bf5a1169fe189562
parent 434419 a327ade4032c0801a1c022b3d40fa44fbecc8290
child 434421 22bcb41cbaec6f8b728360aa3693904be0b7f656
push id8114
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 16:33:21 +0000
treeherdermozilla-beta@73e0d89a540f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersannevk, bkelly
bugs1389274
milestone58.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 1389274 - Correct the behavior of Element.scrollIntoView to match the draft spec and pass web platform tests; r=annevk,bkelly 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