Backed out changeset 8a970e561fe1 (bug 1389274) for unexpected passes of web-platform-test /cssom-view/scrollIntoView-shadow.html. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Thu, 28 Sep 2017 19:46:03 +0200
changeset 436769 489a4a560c60f2eb0f0a145ab01626f798247498
parent 436768 93561ea081ffb2bb4a68a4828a230f95ba201078
child 436770 638e145f6bba437642d55f7b2baf5458df61419a
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1389274
milestone58.0a1
backs out8a970e561fe106adbec7554499292d7fe8d6c1b5
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
Backed out changeset 8a970e561fe1 (bug 1389274) for unexpected passes of web-platform-test /cssom-view/scrollIntoView-shadow.html. r=backout
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-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,99 +723,61 @@ Element::GetScrollFrame(nsIFrame **aStyl
       return shell->GetRootScrollFrameAsScrollable();
     }
   }
 
   return nullptr;
 }
 
 void
-Element::ScrollIntoView(const BooleanOrScrollIntoViewOptions& aObject)
+Element::ScrollIntoView()
 {
-  if (aObject.IsScrollIntoViewOptions()) {
-    return ScrollIntoView(aObject.GetAsScrollIntoViewOptions());
-  }
-
-  MOZ_DIAGNOSTIC_ASSERT(aObject.IsBoolean());
-
+  ScrollIntoView(ScrollIntoViewOptions());
+}
+
+void
+Element::ScrollIntoView(bool aTop)
+{
   ScrollIntoViewOptions options;
-  if (aObject.GetAsBoolean()) {
-    options.mBlock = ScrollLogicalPosition::Start;
-    options.mInline = ScrollLogicalPosition::Nearest;
-  } else {
+  if (!aTop) {
     options.mBlock = ScrollLogicalPosition::End;
-    options.mInline = ScrollLogicalPosition::Nearest;
-  }
-  return ScrollIntoView(options);
+  }
+  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 = 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");
-  }
+  int16_t vpercent = (aOptions.mBlock == ScrollLogicalPosition::Start)
+                       ? nsIPresShell::SCROLL_TOP
+                       : nsIPresShell::SCROLL_BOTTOM;
 
   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(
-                                     hpercent,
-                                     nsIPresShell::SCROLL_ALWAYS),
+                                   nsIPresShell::ScrollAxis(),
                                    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,20 +1074,19 @@ public:
   already_AddRefed<DestinationInsertionPointList> GetDestinationInsertionPoints();
 
   ShadowRoot *FastGetShadowRoot() const
   {
     nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots();
     return slots ? slots->mShadowRoot.get() : nullptr;
   }
 
-private:
+  void ScrollIntoView();
+  void ScrollIntoView(bool aTop);
   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", "center", "end", "nearest" };
+enum ScrollLogicalPosition { "start", "end" };
 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(optional (boolean or ScrollIntoViewOptions) arg);
+  void scrollIntoView(boolean top);
+  void scrollIntoView(optional ScrollIntoViewOptions options);
   // 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,16 +337734,22 @@
     ]
    ],
    "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": [
     [
@@ -578693,16 +578699,20 @@
   "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"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/cssom-view/scrollIntoView-empty-args.html.ini
@@ -0,0 +1,11 @@
+[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
+
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/cssom-view/scrollIntoView-smooth.html.ini
@@ -0,0 +1,11 @@
+[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
+
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/cssom-view/scrollintoview.html.ini
@@ -0,0 +1,38 @@
+[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
+
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/cssom-view/scrollIntoView-empty-args.html
@@ -0,0 +1,83 @@
+<!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