Bug 1351170 - 1. Correctly calculate start offset for non-text nodes; r=masayuki
authorJim Chen <nchen@mozilla.com>
Wed, 19 Jul 2017 14:29:59 -0400
changeset 418393 7773fb9e0bf35ad416aa2c1f785148e48d50f299
parent 418392 17e2e2aa8f56546d6749d41266af06b7390df7db
child 418394 b7909eddfe2659a1d02afe4f416b28a2a21c0b28
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1351170
milestone56.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 1351170 - 1. Correctly calculate start offset for non-text nodes; r=masayuki When the start node is a non-container node (i.e. <br>), and the start offset is 0, we should not include a newline character for the node. For example, for this range, > <br/>hello > \___/ the start node/offset is (<br/>, 0) and end node/offset is ("hello", 1). The calculated range offset should be 0, and the range length should be 2: 1 for the <br/> newline character plus 1 for "h". The patch also ensures this behavior for pre-mode nsContentIterator, for both start and end node adjustments. For start nodes, we include any non-container nodes with offset 0 in the range. For end node, we exclude any non-container nodes with offset 0 from the range. MozReview-Commit-ID: Lt2tCLbapq7
dom/base/nsContentIterator.cpp
dom/events/ContentEventHandler.cpp
widget/tests/window_composition_text_querycontent.xul
--- a/dom/base/nsContentIterator.cpp
+++ b/dom/base/nsContentIterator.cpp
@@ -10,16 +10,17 @@
 #include "nsIContentIterator.h"
 #include "nsRange.h"
 #include "nsIContent.h"
 #include "nsCOMPtr.h"
 #include "nsTArray.h"
 #include "nsContentUtils.h"
 #include "nsINode.h"
 #include "nsCycleCollectionParticipant.h"
+#include "nsIParserService.h"
 
 using mozilla::DebugOnly;
 
 // couple of utility static functs
 
 ///////////////////////////////////////////////////////////////////////////
 // NodeToParentOffset: returns the node's parent and offset.
 //
@@ -366,20 +367,27 @@ nsContentIterator::Init(nsIDOMRange* aDO
     // No children (possibly a <br> or text node), or index is after last child.
 
     if (mPre) {
       // XXX: In the future, if start offset is after the last
       //      character in the cdata node, should we set mFirst to
       //      the next sibling?
 
       // Normally we would skip the start node because the start node is outside
-      // of the range in pre mode. However, if startIndx == 0, it means the node
-      // has no children, and the node may be <br> or something. We don't skip
-      // the node in this case in order to address bug 1215798.
-      if (!startIsData && startIndx) {
+      // of the range in pre mode. However, if startIndx == 0, and the node is a
+      // non-container node (e.g. <br>), we don't skip the node in this case in
+      // order to address bug 1215798.
+      bool startIsContainer = true;
+      if (startNode->IsHTMLElement()) {
+        if (nsIParserService* ps = nsContentUtils::GetParserService()) {
+          nsIAtom* name = startNode->NodeInfo()->NameAtom();
+          ps->IsContainer(ps->HTMLAtomTagToId(name), startIsContainer);
+        }
+      }
+      if (!startIsData && (startIsContainer || startIndx)) {
         mFirst = GetNextSibling(startNode);
         NS_WARNING_ASSERTION(mFirst, "GetNextSibling returned null");
 
         // Does mFirst node really intersect the range?  The range could be
         // 'degenerate', i.e., not collapsed but still contain no content.
         if (mFirst &&
             NS_WARN_IF(!NodeIsInTraversalRange(mFirst, mPre, startNode,
                                                startIndx, endNode, endIndx))) {
@@ -422,24 +430,32 @@ nsContentIterator::Init(nsIDOMRange* aDO
   bool endIsData = endNode->IsNodeOfType(nsINode::eDATA_NODE);
 
   if (endIsData || !endNode->HasChildren() || endIndx == 0) {
     if (mPre) {
       if (NS_WARN_IF(!endNode->IsContent())) {
         // Not much else to do here...
         mLast = nullptr;
       } else {
-        // If the end node is an empty element and the end offset is 0,
+        // If the end node is a non-container element and the end offset is 0,
         // the last element should be the previous node (i.e., shouldn't
         // include the end node in the range).
-        if (!endIsData && !endNode->HasChildren() && !endIndx) {
+        bool endIsContainer = true;
+        if (endNode->IsHTMLElement()) {
+          if (nsIParserService* ps = nsContentUtils::GetParserService()) {
+            nsIAtom* name = endNode->NodeInfo()->NameAtom();
+            ps->IsContainer(ps->HTMLAtomTagToId(name), endIsContainer);
+          }
+        }
+        if (!endIsData && !endIsContainer && !endIndx) {
           mLast = PrevNode(endNode);
           NS_WARNING_ASSERTION(mLast, "PrevNode returned null");
-          if (NS_WARN_IF(!NodeIsInTraversalRange(mLast, mPre,
-                                                 startNode, startIndx,
+          if (mLast && mLast != mFirst &&
+              NS_WARN_IF(!NodeIsInTraversalRange(mLast, mPre,
+                                                 mFirst, 0,
                                                  endNode, endIndx))) {
             mLast = nullptr;
           }
         } else {
           mLast = endNode->AsContent();
         }
       }
     } else {
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -14,16 +14,17 @@
 #include "nsCaret.h"
 #include "nsCOMPtr.h"
 #include "nsContentUtils.h"
 #include "nsCopySupport.h"
 #include "nsFocusManager.h"
 #include "nsFontMetrics.h"
 #include "nsFrameSelection.h"
 #include "nsIContentIterator.h"
+#include "nsIParserService.h"
 #include "nsIPresShell.h"
 #include "nsISelection.h"
 #include "nsIFrame.h"
 #include "nsIObjectFrame.h"
 #include "nsLayoutUtils.h"
 #include "nsPresContext.h"
 #include "nsQueryObject.h"
 #include "nsRange.h"
@@ -2834,19 +2835,43 @@ ContentEventHandler::GetFlatTextLengthIn
 }
 
 nsresult
 ContentEventHandler::GetStartOffset(nsRange* aRange,
                                     uint32_t* aOffset,
                                     LineBreakType aLineBreakType)
 {
   MOZ_ASSERT(aRange);
+  // To match the "no skip start" hack in nsContentIterator::Init, when range
+  // offset is 0 and the range node is not a container, we have to assume the
+  // range _includes_ the node, which means the start offset should _not_
+  // include the node.
+  //
+  // For example, for this content: <br>abc, and range (<br>, 0)-("abc", 1), the
+  // range includes the linebreak from <br>, so the start offset should _not_
+  // include <br>, and the start offset should be 0.
+  //
+  // However, for this content: <p/>abc, and range (<p>, 0)-("abc", 1), the
+  // range does _not_ include the linebreak from <p> because <p> is a container,
+  // so the start offset _should_ include <p>, and the start offset should be 1.
+
+  nsINode* startNode = aRange->GetStartContainer();
+  bool startIsContainer = true;
+  if (startNode->IsHTMLElement()) {
+    if (nsIParserService* ps = nsContentUtils::GetParserService()) {
+      nsIAtom* name = startNode->NodeInfo()->NameAtom();
+      ps->IsContainer(ps->HTMLAtomTagToId(name), startIsContainer);
+    }
+  }
+  const NodePosition& startPos =
+    startIsContainer
+    ? NodePosition(startNode, aRange->StartOffset())
+    : NodePositionBefore(startNode, aRange->StartOffset());
   return GetFlatTextLengthInRange(
-           NodePosition(mRootContent, 0),
-           NodePosition(aRange->GetStartContainer(), aRange->StartOffset()),
+           NodePosition(mRootContent, 0), startPos,
            mRootContent, aOffset, aLineBreakType);
 }
 
 nsresult
 ContentEventHandler::AdjustCollapsedRangeMaybeIntoTextNode(nsRange* aRange)
 {
   MOZ_ASSERT(aRange);
   MOZ_ASSERT(aRange->Collapsed());
--- a/widget/tests/window_composition_text_querycontent.xul
+++ b/widget/tests/window_composition_text_querycontent.xul
@@ -4281,16 +4281,38 @@ function runQueryTextContentEventTest()
 
   // #16
   contenteditable.innerHTML = "a<blink>b</blink>c";
 
   result = synthesizeQueryTextContent(0, 3);
   is(result.text, "abc", "runQueryTextContentEventTest #16 (0, 3), \"" + contenteditable.innerHTML + "\"");
 }
 
+function runQuerySelectionEventTest()
+{
+  contenteditable.focus();
+
+  var selection = windowOfContenteditable.getSelection();
+
+  // #1
+  contenteditable.innerHTML = "<br/>a";
+  selection.setBaseAndExtent(contenteditable.firstChild, 0, contenteditable.lastChild, 1);
+  checkSelection(0, kLF + "a", "runQuerySelectionEventTest #1, \"" + contenteditable.innerHTML + "\"");
+
+  // #2
+  contenteditable.innerHTML = "<p></p><p>abc</p>";
+  selection.setBaseAndExtent(contenteditable.firstChild, 0, contenteditable.lastChild.firstChild, 1);
+  checkSelection(kLFLen, kLF + "a", "runQuerySelectionEventTest #2, \"" + contenteditable.innerHTML + "\"");
+
+  // #3
+  contenteditable.innerHTML = "<p>abc</p><p>def</p>";
+  selection.setBaseAndExtent(contenteditable.firstChild, 0, contenteditable.lastChild.firstChild, 1);
+  checkSelection(kLFLen, "abc" + kLF + "d", "runQuerySelectionEventTest #3, \"" + contenteditable.innerHTML + "\"");
+}
+
 function runQueryIMESelectionTest()
 {
   textarea.focus();
   textarea.value = "before  after";
   var startoffset = textarea.selectionStart = textarea.selectionEnd = "before ".length;
 
   if (!checkIMESelection("RawClause", false, 0, "", "runQueryIMESelectionTest: before starting composition") ||
       !checkIMESelection("SelectedRawClause", false, 0, "", "runQueryIMESelectionTest: before starting composition") ||
@@ -8115,16 +8137,17 @@ function* testBody()
   runCompositionCommitTest();
   runCompositionTest();
   runCompositionEventTest();
   runQueryTextRectInContentEditableTest();
   runCharAtPointTest(textarea, "textarea in the document");
   runCharAtPointAtOutsideTest();
   runSetSelectionEventTest();
   runQueryTextContentEventTest();
+  runQuerySelectionEventTest();
   runQueryIMESelectionTest();
   runQueryContentEventRelativeToInsertionPoint();
   yield* runIMEContentObserverTest();
   runCSSTransformTest();
   runBug722639Test();
   runBug1375825Test();
   runForceCommitTest();
   runNestedSettingValue();