Bug 1482425 - PresShell::PageMove() should use different rules to look for a container element for aExtend value r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 18 Oct 2018 04:42:53 +0000
changeset 490205 02a07fe8780872236c65ddc30470209bef95b71b
parent 490204 afdfeb4c004cbba91d4d9e30a0e80e6dfb0dce60
child 490206 e37adb23fd48ca1576ba954fe203b3fd6d6155f3
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerssmaug
bugs1482425, 1369072
milestone64.0a1
Bug 1482425 - PresShell::PageMove() should use different rules to look for a container element for aExtend value r=smaug PresShell::PageMove() climbs up to parent document when there is no scrollable parent in current document. However, if aExtend is true, it should expand Selection in the document itself. Therefore, it needs different rules to look for container of expanding Selection from scrollable element to scroll. Additionally, old rules (i.e., before the fix of bug 1369072 which caused this regression) were also buggy. It used parent scrollable element or root scrollable element simply. Therefore, if found scrollable element is ancestor of selection limiter, it didn't work as expected. This patch creates nsFrameSelection::GetFrameToPageSelect() to retrieve per-page selection container element with the following rules: - look for a scrollable element in selection limiter. - if there is no scrollable element, use selection limiter. - if there is no selection limiter, use the root frame. So, nsFrameSelection::CommonPageMove() should take nsIFrame rather than nsIScrollableFrame since container of per-page selection may be used in non-scrollable contenteditable element. If it's called with non-scrollable frame, it needs to compute the expanding range with the frame size. Differential Revision: https://phabricator.services.mozilla.com/D8954
dom/html/nsTextEditorState.cpp
layout/base/PresShell.cpp
layout/base/PresShell.h
layout/base/tests/mochitest.ini
layout/base/tests/test_expanding_selection_per_page.html
layout/base/tests/window_empty_document.html
layout/generic/nsFrameSelection.cpp
layout/generic/nsFrameSelection.h
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -336,16 +336,17 @@ public:
   NS_IMETHOD PhysicalMove(int16_t aDirection, int16_t aAmount, bool aExtend) override;
   NS_IMETHOD CharacterMove(bool aForward, bool aExtend) override;
   NS_IMETHOD CharacterExtendForDelete() override;
   NS_IMETHOD CharacterExtendForBackspace() override;
   NS_IMETHOD WordMove(bool aForward, bool aExtend) override;
   NS_IMETHOD WordExtendForDelete(bool aForward) override;
   NS_IMETHOD LineMove(bool aForward, bool aExtend) override;
   NS_IMETHOD IntraLineMove(bool aForward, bool aExtend) override;
+  MOZ_CAN_RUN_SCRIPT
   NS_IMETHOD PageMove(bool aForward, bool aExtend) override;
   NS_IMETHOD CompleteScroll(bool aForward) override;
   NS_IMETHOD CompleteMove(bool aForward, bool aExtend) override;
   NS_IMETHOD ScrollPage(bool aForward) override;
   NS_IMETHOD ScrollLine(bool aForward) override;
   NS_IMETHOD ScrollCharacter(bool aRight) override;
   NS_IMETHOD SelectAll(void) override;
   NS_IMETHOD CheckVisibility(nsINode *node, int16_t startOffset, int16_t EndOffset, bool* _retval) override;
@@ -672,20 +673,20 @@ nsTextInputSelectionImpl::IntraLineMove(
 }
 
 
 NS_IMETHODIMP
 nsTextInputSelectionImpl::PageMove(bool aForward, bool aExtend)
 {
   // expected behavior for PageMove is to scroll AND move the caret
   // and to remain relative position of the caret in view. see Bug 4302.
-  if (mScrollFrame)
-  {
+  if (mScrollFrame) {
     RefPtr<nsFrameSelection> frameSelection = mFrameSelection;
-    frameSelection->CommonPageMove(aForward, aExtend, mScrollFrame);
+    nsIFrame* scrollFrame = do_QueryFrame(mScrollFrame);
+    frameSelection->CommonPageMove(aForward, aExtend, scrollFrame);
   }
   // After ScrollSelectionIntoView(), the pending notifications might be
   // flushed and PresShell/PresContext/Frames may be dead. See bug 418470.
   return ScrollSelectionIntoView(nsISelectionController::SELECTION_NORMAL,
                                  nsISelectionController::SELECTION_FOCUS_REGION,
                                  nsISelectionController::SCROLL_SYNCHRONOUS |
                                  nsISelectionController::SCROLL_FOR_CARET_MOVE);
 }
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -2324,23 +2324,27 @@ PresShell::IntraLineMove(bool aForward, 
   return frameSelection->IntraLineMove(aForward, aExtend);
 }
 
 
 
 NS_IMETHODIMP
 PresShell::PageMove(bool aForward, bool aExtend)
 {
-  nsIScrollableFrame *scrollableFrame =
-    GetScrollableFrameToScroll(nsIPresShell::eVertical);
-  if (!scrollableFrame)
+  nsIFrame* frame;
+  if (!aExtend) {
+    frame = do_QueryFrame(GetScrollableFrameToScroll(nsIPresShell::eVertical));
+  } else {
+    frame = mSelection->GetFrameToPageSelect();
+  }
+  if (!frame) {
     return NS_OK;
-
+  }
   RefPtr<nsFrameSelection> frameSelection = mSelection;
-  frameSelection->CommonPageMove(aForward, aExtend, scrollableFrame);
+  frameSelection->CommonPageMove(aForward, aExtend, frame);
   // After ScrollSelectionIntoView(), the pending notifications might be
   // flushed and PresShell/PresContext/Frames may be dead. See bug 418470.
   return ScrollSelectionIntoView(nsISelectionController::SELECTION_NORMAL,
                                  nsISelectionController::SELECTION_FOCUS_REGION,
                                  nsISelectionController::SCROLL_SYNCHRONOUS |
                                  nsISelectionController::SCROLL_FOR_CARET_MOVE);
 }
 
--- a/layout/base/PresShell.h
+++ b/layout/base/PresShell.h
@@ -266,16 +266,17 @@ public:
   NS_IMETHOD PhysicalMove(int16_t aDirection, int16_t aAmount, bool aExtend) override;
   NS_IMETHOD CharacterMove(bool aForward, bool aExtend) override;
   NS_IMETHOD CharacterExtendForDelete() override;
   NS_IMETHOD CharacterExtendForBackspace() override;
   NS_IMETHOD WordMove(bool aForward, bool aExtend) override;
   NS_IMETHOD WordExtendForDelete(bool aForward) override;
   NS_IMETHOD LineMove(bool aForward, bool aExtend) override;
   NS_IMETHOD IntraLineMove(bool aForward, bool aExtend) override;
+  MOZ_CAN_RUN_SCRIPT
   NS_IMETHOD PageMove(bool aForward, bool aExtend) override;
   NS_IMETHOD ScrollPage(bool aForward) override;
   NS_IMETHOD ScrollLine(bool aForward) override;
   NS_IMETHOD ScrollCharacter(bool aRight) override;
   NS_IMETHOD CompleteScroll(bool aForward) override;
   NS_IMETHOD CompleteMove(bool aForward, bool aExtend) override;
   NS_IMETHOD SelectAll() override;
   NS_IMETHOD CheckVisibility(nsINode *node, int16_t startOffset, int16_t EndOffset, bool *_retval) override;
--- a/layout/base/tests/mochitest.ini
+++ b/layout/base/tests/mochitest.ini
@@ -146,16 +146,18 @@ support-files = bug1226904.html
 [test_bug1246622.html]
 [test_bug1278021.html]
 [test_emulateMedium.html]
 [test_event_target_iframe_oop.html]
 skip-if = e10s # bug 1020135, nested oop iframes not supported
 support-files = bug921928_event_target_iframe_apps_oop.html
 [test_event_target_radius.html]
 skip-if = toolkit == 'android' # Bug 1355836
+[test_expanding_selection_per_page.html]
+support-files = window_empty_document.html
 [test_flush_on_paint.html]
 skip-if = true # Bug 688128
 [test_frame_reconstruction_for_pseudo_elements.html]
 [test_frame_reconstruction_for_svg_transforms.html]
 [test_frame_reconstruction_scroll_restore.html]
 [test_getBoxQuads_convertPointRectQuad.html]
 support-files =
   file_getBoxQuads_convertPointRectQuad_frame1.html
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/test_expanding_selection_per_page.html
@@ -0,0 +1,310 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Test for expanding selection per page</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+
+<pre id="test">
+<script class="testbody" type="text/javascript">
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(() => {
+  open("window_empty_document.html", "_blank", "width=500,height=500");
+});
+
+async function doTests(aWindow) {
+  const IS_WIN = navigator.platform.includes("Win");
+  // On macOS and Linux, Shift + PageUp/PageDown requires native event to
+  // resolve default action of PageDown and PageUp. Although macOS widget has
+  // nsIWidget::AttachNativeKeyEvent(), we cannot use synthesizeKey() for the
+  // following tests.  So, use nsISelectionController.pageMove() instead on
+  // non-Windows platforms.
+  const kUseKeyboardEvent = IS_WIN;
+  let selectionController;
+  if (!kUseKeyboardEvent) {
+    selectionController = SpecialPowers.wrap(aWindow)
+                                       .docShell
+                                       .QueryInterface(SpecialPowers.Ci.nsIInterfaceRequestor)
+                                       .getInterface(SpecialPowers.Ci.nsISelectionDisplay)
+                                       .QueryInterface(SpecialPowers.Ci.nsISelectionController);
+  }
+  // On Windows, per-page selection to start or end expands selection to same
+  // column of first or last line.  On the other platforms, it expands selection
+  // to start or end of first or last line.
+  const kSelectToStartOrEnd = !IS_WIN;
+
+  await SpecialPowers.pushPrefEnv({"set": [["general.smoothScroll", false]]});
+
+  function getNodeDescription(aNode) {
+    function getElementDescription(aElement) {
+      if (aElement.getAttribute("id") !== null) {
+        return `${aElement.tagName.toLowerCase()}#${aElement.getAttribute("id")}`;
+      }
+      if (aElement.tagName === "BR") {
+        return `${getElementDescription(aElement.previousSibling)} + br`;
+      }
+      return aElement.tagName.toLowerCase();
+    }
+    switch (aNode.nodeType) {
+      case aNode.TEXT_NODE:
+        return `text node in ${getElementDescription(aNode.parentElement)}`;
+      case aNode.ELEMENT_NODE:
+        return getElementDescription(aNode);
+      default:
+        return "unknown node";
+    }
+  }
+
+  function doSelectPageDown() {
+    if (kUseKeyboardEvent) {
+      synthesizeKey("KEY_PageDown", {shiftKey: true}, aWindow);
+    } else {
+      selectionController.pageMove(true, true);
+    }
+  }
+
+  function doSelectPageUp() {
+    if (kUseKeyboardEvent) {
+      synthesizeKey("KEY_PageUp", {shiftKey: true}, aWindow);
+    } else {
+      selectionController.pageMove(false, true);
+    }
+  }
+
+  let doc = aWindow.document;
+  let body = doc.body;
+  let selection = doc.getSelection();
+  let container;
+
+  body.innerHTML = '<span id="s1">first line</span><br>' +
+                   '<span id="s2">second line</span><br>' +
+                   '<span id="s3">last line</span>';
+  container = doc.documentElement;
+
+  let description = "Expanding selection to forward in non-scrollable body: ";
+  is(container.scrollTop, 0, description + "scrollTop should be 0 at initialization");
+  selection.collapse(doc.getElementById("s1").firstChild, 3);
+  doSelectPageDown();
+  is(container.scrollTop, 0, description + "this test shouldn't create scrollable document");
+  let range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s1").firstChild,
+     `${description} selection should be expanded from the first line (got: ${getNodeDescription(range.startContainer)})`);
+  is(range.startOffset, 3,
+     `${description} selection should be expanded from the first line's 3rd insertion point`);
+  is(range.endContainer, doc.getElementById("s3").firstChild,
+     `${description} selection should be expanded into the last line (got: ${getNodeDescription(range.endContainer)})`);
+  if (kSelectToStartOrEnd) {
+    is(range.endOffset, range.endContainer.length,
+       `${description} selection should be expanded to end of the last line`);
+  } else {
+    isfuzzy(range.endOffset, 3, 2,
+            `${description} selection should be expanded to around the last line's 3rd insertion point`);
+  }
+
+  description = "Expanding selection to backward in non-scrollable body: ";
+  selection.collapse(doc.getElementById("s3").firstChild, 3);
+  doSelectPageUp();
+  is(container.scrollTop, 0, description + "this test shouldn't create scrollable document");
+  range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s1").firstChild,
+     `${description} selection should be expanded into the first line (got: ${getNodeDescription(range.startContainer)})`);
+  if (kSelectToStartOrEnd) {
+    is(range.startOffset, 0,
+       `${description} selection should be expanded to start of the first line`);
+  } else {
+    isfuzzy(range.startOffset, 3, 2,
+            `${description} selection should be expanded to around the first line's 3rd insertion point`);
+  }
+  is(range.endContainer, doc.getElementById("s3").firstChild,
+     `${description} selection should be expanded from the last line (got: ${getNodeDescription(range.endContainer)})`);
+  is(range.endOffset, 3,
+     `${description} selection should be expanded from the last line's 3rd insertion point`);
+
+  body.innerHTML = '<span id="s1">first line in the body</span>' +
+                     '<div id="d1" style="height: 2em; line-height: 1em; overflow: auto;">' +
+                       '<span id="s2">first line</span><br>' +
+                       '<span id="s3">second line</span><br>' +
+                       '<span id="s4">third line</span><br>' +
+                       '<span id="s5">last line</span>' +
+                     "</div>" +
+                   '<span id="s6">last line in the body</span>';
+  container = doc.getElementById("d1");
+
+  description = "Expanding selection to forward in scrollable area in the body: ";
+  is(container.scrollTop, 0, description + "scrollTop should be 0 at initialization");
+  selection.collapse(doc.getElementById("s2").firstChild, 3);
+  doSelectPageDown();
+  isnot(container.scrollTop, 0, description + "should be scrolled down");
+  range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s2").firstChild,
+     `${description} selection should be expanded from the first line (got: ${getNodeDescription(range.startContainer)})`);
+  is(range.startOffset, 3,
+     `${description} selection should be expanded from the first line's 3rd insertion point`);
+  is(range.endContainer, doc.getElementById("s4").firstChild,
+     `${description} selection should be expanded into the 3rd line (got: ${getNodeDescription(range.endContainer)})`);
+  isfuzzy(range.endOffset, 3, 2,
+          `${description} selection should be expanded to around the 3rd line's 3rd insertion point`);
+
+  description = "Expanding selection to backward in scrollable area in the body: ";
+  selection.collapse(doc.getElementById("s4").firstChild, 3);
+  let previousScrollTop = container.scrollTop;
+  doSelectPageUp();
+  ok(container.scrollTop < previousScrollTop, description + "should be scrolled up");
+  range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s2").firstChild,
+     `${description} selection should be expanded into the first line (got: ${getNodeDescription(range.startContainer)})`);
+  isfuzzy(range.startOffset, 3, 2,
+          `${description} selection should be expanded to around the first line's 3rd insertion point`);
+  is(range.endContainer, doc.getElementById("s4").firstChild,
+     `${description} selection should be expanded from the 3rd line (got: ${getNodeDescription(range.endContainer)})`);
+  is(range.endOffset, 3,
+     `${description} selection should be expanded from the 3rd line's 3rd insertion point`);
+
+  body.innerHTML = '<span id="s1">first line in the body</span>' +
+                     '<div id="d1" contenteditable style="height: 2em; line-height: 1em; overflow: auto;">' +
+                       '<span id="s2">first line</span><br>' +
+                       '<span id="s3">second line</span><br>' +
+                       '<span id="s4">third line</span><br>' +
+                       '<span id="s5">last line</span>' +
+                     "</div>" +
+                   '<span id="s6">last line in the body</span>';
+  container = doc.getElementById("d1");
+
+  description = "Expanding selection to forward in scrollable editable div in the body: ";
+  is(container.scrollTop, 0, description + "scrollTop should be 0 at initialization");
+  selection.collapse(doc.getElementById("s2").firstChild, 3);
+  doSelectPageDown();
+  isnot(container.scrollTop, 0, description + "should be scrolled down");
+  range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s2").firstChild,
+     `${description} selection should be expanded from the first line (got: ${getNodeDescription(range.startContainer)})`);
+  is(range.startOffset, 3,
+     `${description} selection should be expanded from the first line's 3rd insertion point`);
+  is(range.endContainer, doc.getElementById("s4").firstChild,
+     `${description} selection should be expanded into the 3rd line (got: ${getNodeDescription(range.endContainer)})`);
+  isfuzzy(range.endOffset, 3, 2,
+          `${description} selection should be expanded to around the 3rd line's 3rd insertion point`);
+
+  description = "Expanding selection to backward in scrollable editable div in the body: ";
+  selection.collapse(doc.getElementById("s4").firstChild, 3);
+  previousScrollTop = container.scrollTop;
+  doSelectPageUp();
+  ok(container.scrollTop < previousScrollTop, description + "should be scrolled up");
+  range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s2").firstChild,
+     `${description} selection should be expanded into the first line (got: ${getNodeDescription(range.startContainer)})`);
+  isfuzzy(range.startOffset, 3, 2,
+          `${description} selection should be expanded to around the first line's 3rd insertion point`);
+  is(range.endContainer, doc.getElementById("s4").firstChild,
+     `${description} selection should be expanded from the 3rd line (got: ${getNodeDescription(range.endContainer)})`);
+  is(range.endOffset, 3,
+     `${description} selection should be expanded from the 3rd line's 3rd insertion point`);
+
+  body.innerHTML = '<span id="s1">first line in the body</span>' +
+                     '<div id="d1" contenteditable>' +
+                       '<span id="s2">first line</span><br>' +
+                       '<span id="s3">second line</span><br>' +
+                       '<span id="s4">third line</span><br>' +
+                       '<span id="s5">last line</span>' +
+                     "</div>" +
+                   '<span id="s6">last line in the body</span>';
+  container = doc.getElementById("d1");
+
+  description = "Expanding selection to forward in non-scrollable editable div in the body: ";
+  is(container.scrollTop, 0, description + "scrollTop should be 0 at initialization");
+  selection.collapse(doc.getElementById("s2").firstChild, 3);
+  doSelectPageDown();
+  is(container.scrollTop, 0, description + "editable div shouldn't be scrollable");
+  range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s2").firstChild,
+     `${description} selection should be expanded from the first line (got: ${getNodeDescription(range.startContainer)})`);
+  is(range.startOffset, 3,
+     `${description} selection should be expanded from the first line's 3rd insertion point`);
+  is(range.endContainer, doc.getElementById("s5").firstChild,
+     `${description} selection should be expanded into the last line (got: ${getNodeDescription(range.endContainer)})`);
+  if (kSelectToStartOrEnd) {
+    is(range.endOffset, range.endContainer.length,
+       `${description} selection should be expanded to end of the last line`);
+  } else {
+    isfuzzy(range.endOffset, 3, 2,
+            `${description} selection should be expanded to around the last line's 3rd insertion point`);
+  }
+
+  description = "Expanding selection to backward in non-scrollable editable div in the body: ";
+  selection.collapse(doc.getElementById("s5").firstChild, 3);
+  doSelectPageUp();
+  is(container.scrollTop, 0, description + "editable div shouldn't be scrollable");
+  range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s2").firstChild,
+     `${description} selection should be expanded into the first line (got: ${getNodeDescription(range.startContainer)})`);
+  if (kSelectToStartOrEnd) {
+    is(range.startOffset, 0,
+       `${description} selection should be expanded to start of the first line`);
+  } else {
+    isfuzzy(range.startOffset, 3, 2,
+            `${description} selection should be expanded to around the first line's 3rd insertion point`);
+  }
+  is(range.endContainer, doc.getElementById("s5").firstChild,
+     `${description} selection should be expanded from the last line (got: ${getNodeDescription(range.endContainer)})`);
+  is(range.endOffset, 3,
+     `${description} selection should be expanded from the last line's 3rd insertion point`);
+
+  body.innerHTML = '<span id="s1">first line in the body</span>' +
+                     '<div id="d1" contenteditable>' +
+                       '<span id="s2">first editable line</span><br>' +
+                       '<div id="d2" style="height: 3em; line-height: 1em; overflow: auto;">' +
+                         '<span id="s3">first line</span><br>' +
+                         '<span id="s4">second line</span>' +
+                       "</div>" +
+                       '<span id="s5">last editable line</span>' +
+                     "</div>" +
+                   '<span id="s6">last line in the body</span>';
+  container = doc.getElementById("d2");
+
+  description = "Expanding selection to forward in scrollable div (but not scrollable along y-axis) in the editable div: ";
+  is(container.scrollTop, 0, description + "scrollTop should be 0 at initialization");
+  selection.collapse(doc.getElementById("s3").firstChild, 3);
+  doSelectPageDown();
+  is(container.scrollTop, 0, description + "scrollable div in the editable div (but not scrollable along y-axis) shouldn't be scrollable");
+  range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s3").firstChild,
+     `${description} selection should be expanded from the first line (got: ${getNodeDescription(range.startContainer)})`);
+  is(range.startOffset, 3,
+     `${description} selection should be expanded from the first line's 3rd insertion point`);
+  is(range.endContainer, doc.getElementById("s5").firstChild,
+     `${description} selection should be expanded into the last editable line (got: ${getNodeDescription(range.endContainer)})`);
+  if (kSelectToStartOrEnd) {
+    is(range.endOffset, range.endContainer.length,
+       `${description} selection should be expanded to end of the last editable line`);
+  } else {
+    isfuzzy(range.endOffset, 3, 2,
+            `${description} selection should be expanded to around the last editable line's 3rd insertion point`);
+  }
+
+  description = "Expanding selection to backward in scrollable div (but not scrollable along y-axis) in the editable div: ";
+  selection.collapse(doc.getElementById("s4").firstChild, 3);
+  doSelectPageUp();
+  is(container.scrollTop, 0, description + "scrollable div (but not scrollable along y-axis) in the editable div shouldn't be scrollable");
+  range = selection.getRangeAt(0);
+  is(range.startContainer, doc.getElementById("s2").firstChild,
+     `${description} selection should be expanded into the first editable line (got: ${getNodeDescription(range.startContainer)})`);
+  if (kSelectToStartOrEnd) {
+    is(range.startOffset, 0,
+       `${description} selection should be expanded to start of the first editable line`);
+  } else {
+    isfuzzy(range.startOffset, 3, 2,
+            `${description} selection should be expanded to around the first editable line's 3rd insertion point`);
+  }
+  is(range.endContainer, doc.getElementById("s4").firstChild,
+     `${description} selection should be expanded from the last line (got: ${getNodeDescription(range.endContainer)})`);
+  is(range.endOffset, 3,
+     `${description} selection should be expanded from the last line's 3rd insertion point`);
+
+  aWindow.close();
+  SimpleTest.finish();
+}
+</script>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/window_empty_document.html
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Empty document to test something in new window</title>
+</head>
+<body onload="opener.doTests(window);"></body>
+</html>
--- a/layout/generic/nsFrameSelection.cpp
+++ b/layout/generic/nsFrameSelection.cpp
@@ -1707,66 +1707,140 @@ nsFrameSelection::GetFrameForNodeOffset(
   }
 
   // find the child frame containing the offset we want
   returnFrame->GetChildFrameContainingOffset(*aReturnOffset, aHint == CARET_ASSOCIATE_AFTER,
                                              &aOffset, &returnFrame);
   return returnFrame;
 }
 
+nsIFrame*
+nsFrameSelection::GetFrameToPageSelect() const
+{
+  if (NS_WARN_IF(!mShell)) {
+    return nullptr;
+  }
+
+  nsIFrame* rootFrameToSelect;
+  if (mLimiter) {
+    rootFrameToSelect = mLimiter->GetPrimaryFrame();
+    if (NS_WARN_IF(!rootFrameToSelect)) {
+      return nullptr;
+    }
+  } else if (mAncestorLimiter) {
+    rootFrameToSelect = mAncestorLimiter->GetPrimaryFrame();
+    if (NS_WARN_IF(!rootFrameToSelect)) {
+      return nullptr;
+    }
+  } else {
+    rootFrameToSelect = mShell->GetRootScrollFrame();
+    if (NS_WARN_IF(!rootFrameToSelect)) {
+      return nullptr;
+    }
+  }
+
+  nsCOMPtr<nsIContent> contentToSelect = mShell->GetContentForScrolling();
+  if (contentToSelect) {
+    // If there is selected content, look for nearest and vertical scrollable
+    // parent under the root frame.
+    for (nsIFrame* frame = contentToSelect->GetPrimaryFrame();
+         frame && frame != rootFrameToSelect;
+         frame = frame->GetParent()) {
+      nsIScrollableFrame* scrollableFrame = do_QueryFrame(frame);
+      if (!scrollableFrame) {
+        continue;
+      }
+      ScrollStyles scrollStyles = scrollableFrame->GetScrollStyles();
+      if (scrollStyles.mVertical == NS_STYLE_OVERFLOW_HIDDEN) {
+        continue;
+      }
+      uint32_t directions = scrollableFrame->GetPerceivedScrollingDirections();
+      if (directions & nsIScrollableFrame::VERTICAL) {
+        // If there is sub scrollable frame, let's use its page size to select.
+        return frame;
+      }
+    }
+  }
+  // Otherwise, i.e., there is no scrollable frame or only the root frame is
+  // scrollable, let's return the root frame because Shift + PageUp/PageDown
+  // should expand the selection in the root content even if it's not
+  // scrollable.
+  return rootFrameToSelect;
+}
+
 void
 nsFrameSelection::CommonPageMove(bool aForward,
                                  bool aExtend,
-                                 nsIScrollableFrame* aScrollableFrame)
+                                 nsIFrame* aFrame)
 {
+  MOZ_ASSERT(aFrame);
+
   // expected behavior for PageMove is to scroll AND move the caret
   // and remain relative position of the caret in view. see Bug 4302.
 
-  //get the frame from the scrollable view
-
-  nsIFrame* scrolledFrame = aScrollableFrame->GetScrolledFrame();
-  if (!scrolledFrame)
+  // Get the scrollable frame.  If aFrame is not scrollable, this is nullptr.
+  nsIScrollableFrame* scrollableFrame = aFrame->GetScrollTargetFrame();
+  // Get the scrolled frame.  If aFrame is not scrollable, this is aFrame
+  // itself.
+  nsIFrame* scrolledFrame =
+    scrollableFrame ? scrollableFrame->GetScrolledFrame() : aFrame;
+  if (!scrolledFrame) {
     return;
+  }
 
   // find out where the caret is.
-  // we should know mDesiredPos value of nsFrameSelection, but I havent seen that behavior in other windows applications yet.
+  // we should know mDesiredPos value of nsFrameSelection, but I havent seen
+  // that behavior in other windows applications yet.
   Selection* domSel = GetSelection(SelectionType::eNormal);
   if (!domSel) {
     return;
   }
 
   nsRect caretPos;
   nsIFrame* caretFrame = nsCaret::GetGeometry(domSel, &caretPos);
-  if (!caretFrame)
+  if (!caretFrame) {
     return;
-
-  //need to adjust caret jump by percentage scroll
-  nsSize scrollDelta = aScrollableFrame->GetPageScrollAmount();
-
-  if (aForward)
-    caretPos.y += scrollDelta.height;
-  else
-    caretPos.y -= scrollDelta.height;
+  }
+
+  if (scrollableFrame) {
+    // If aFrame is scrollable, adjust pseudo-click position with page scroll
+    // amount.
+    if (aForward) {
+      caretPos.y += scrollableFrame->GetPageScrollAmount().height;
+    } else {
+      caretPos.y -= scrollableFrame->GetPageScrollAmount().height;
+    }
+  } else {
+    // Otherwise, adjust pseudo-click position with the frame size.
+    if (aForward) {
+      caretPos.y += scrolledFrame->GetSize().height;
+    } else {
+      caretPos.y -= scrolledFrame->GetSize().height;
+    }
+  }
 
   caretPos += caretFrame->GetOffsetTo(scrolledFrame);
 
   // get a content at desired location
   nsPoint desiredPoint;
   desiredPoint.x = caretPos.x;
-  desiredPoint.y = caretPos.y + caretPos.height/2;
+  desiredPoint.y = caretPos.y + caretPos.height / 2;
   nsIFrame::ContentOffsets offsets =
       scrolledFrame->GetContentOffsetsFromPoint(desiredPoint);
 
-  if (!offsets.content)
+  if (!offsets.content) {
     return;
-
-  // scroll one page
-  aScrollableFrame->ScrollBy(nsIntPoint(0, aForward ? 1 : -1),
-                             nsIScrollableFrame::PAGES,
-                             nsIScrollableFrame::SMOOTH);
+  }
+
+  // Scroll one page if necessary.
+  if (scrollableFrame) {
+    scrollableFrame->ScrollBy(nsIntPoint(0, aForward ? 1 : -1),
+                               nsIScrollableFrame::PAGES,
+                               nsIScrollableFrame::SMOOTH);
+  }
 
   // place the caret
   HandleClick(offsets.content, offsets.offset,
               offsets.offset, aExtend, false, CARET_ASSOCIATE_AFTER);
 }
 
 nsresult
 nsFrameSelection::PhysicalMove(int16_t aDirection, int16_t aAmount,
--- a/layout/generic/nsFrameSelection.h
+++ b/layout/generic/nsFrameSelection.h
@@ -420,31 +420,41 @@ public:
    * @param aReturnOffset will contain offset into frame.
    */
   nsIFrame* GetFrameForNodeOffset(nsIContent*        aNode,
                                   int32_t            aOffset,
                                   CaretAssociateHint aHint,
                                   int32_t*           aReturnOffset) const;
 
   /**
+   * GetFrameToPageSelect() returns a frame which is ancestor limit of
+   * per-page selection.  The frame may not be scrollable.  E.g.,
+   * when selection ancestor limit is set to a frame of an editing host of
+   * contenteditable element and it's not scrollable.
+   */
+  nsIFrame* GetFrameToPageSelect() const;
+
+  /**
    * Scrolling then moving caret placement code in common to text areas and
    * content areas should be located in the implementer
    * This method will accept the following parameters and perform the scroll
    * and caret movement.  It remains for the caller to call the final
    * ScrollCaretIntoView if that called wants to be sure the caret is always
    * visible.
    *
    * @param aForward if true, scroll forward if not scroll backward
    * @param aExtend  if true, extend selection to the new point
-   * @param aScrollableFrame the frame to scroll
+   * @param aFrame   the frame to scroll or container of per-page selection.
+   *                 if aExtend is true and selection may have ancestor limit,
+   *                 should set result of GetFrameToPageSelect().
    */
-  /*unsafe*/
+  MOZ_CAN_RUN_SCRIPT
   void CommonPageMove(bool aForward,
                       bool aExtend,
-                      nsIScrollableFrame* aScrollableFrame);
+                      nsIFrame* aFrame);
 
   void SetHint(CaretAssociateHint aHintRight) { mHint = aHintRight; }
   CaretAssociateHint GetHint() const { return mHint; }
 
   /**
    * SetCaretBidiLevel sets the caret bidi level.
    * @param aLevel the caret bidi level
    */