Generalize the the special handling of HR tags in

goog.editor.range.placeCursorNextTo() to better handle IMG tags and
other no-child-allowed Element nodes. This logic now takes effect
for all browsers, rather than just IE/IE9.
The the original behavior caused scrolling issues
when inserting images or emoji in gmail, on IE.
Also change (for IE) the order in which we set focus and restore
the cursor in abstractDialogPlugin.
This fixes the scroll-to-the-top-after-insert behavior on IE, 
as this new set of calls doesn't spoil the
restoration of the proper scroll position.

R=nicksantos
DELTA=209 (167 added, 18 deleted, 24 changed)


Revision created by MOE tool push_codebase.
MOE_MIGRATION=6062


git-svn-id: http://closure-library.googlecode.com/svn/trunk@2407 0b95b8e8-c90f-11de-9d4f-f947ee5921c8

closure/goog/editor/field.js

@@ -2330,6 +2330,24 @@ goog.editor.Field.prototype.placeCursorAtStartOrEnd_ = function(isStart) {
 };
 
 
+/**
+ * Restore a saved range, and set the focus on the field.
+ * If no range is specified, we simply set the focus.
+ * @param {goog.dom.SavedRange=} opt_range A previously saved selected range.
+ */
+goog.editor.Field.prototype.restoreSavedRange = function(opt_range) {
+  if (goog.userAgent.IE) {
+    this.focus();
+  }
+  if (opt_range) {
+    opt_range.restore();
+  }
+  if (!goog.userAgent.IE) {
+    this.focus();
+  }
+};
+
+
 /**
  * Makes a field editable.
  *

closure/goog/editor/field_test.js

@@ -1014,6 +1014,45 @@ function doTestPlaceCursorAtStart(opt_html, opt_parentId) {
 }
 
 
+/**
+ * Verify that restoreSavedRange() restores the range and sets the focus.
+ */
+function testRestoreSavedRange() {
+  var editableField = new FieldConstructor('testField', document);
+  editableField.makeEditable();
+
+
+  // Create another node to take the focus later.
+  var doc = goog.dom.getOwnerDocument(editableField.getElement());
+  var otherElem = doc.createElement('div');
+  otherElem.tabIndex = '1';  // Make it focusable.
+  editableField.getElement().parentNode.appendChild(otherElem);
+
+  // Initially place selection not at the start of the editable field.
+  var textNode = editableField.getElement().firstChild;
+  var range = goog.dom.Range.createFromNodes(textNode, 1, textNode, 2);
+  range.select();
+  var savedRange = goog.editor.range.saveUsingNormalizedCarets(range);
+
+  // Change range to be a simple cursor at start, and move focus away.
+  editableField.placeCursorAtStart();
+  otherElem.focus();
+
+  editableField.restoreSavedRange(savedRange);
+
+  // Verify that we have focus and the range is restored.
+  assertEquals('Field should be focused',
+      editableField.getElement(),
+      goog.dom.getActiveElement(doc));
+  var newRange = editableField.getRange();
+  assertEquals('Range startNode', textNode, newRange.getStartNode());
+  assertEquals('Range startOffset', 1, newRange.getStartOffset());
+  assertEquals('Range endNode', textNode, newRange.getEndNode());
+  assertEquals('Range endOffset', 2, newRange.getEndOffset());
+
+}
+
+
 function testPlaceCursorAtStart() {
   doTestPlaceCursorAtStart();
 }
@@ -1062,8 +1101,10 @@ function doTestPlaceCursorAtEnd(opt_html, opt_parentId, opt_offset) {
 
   // We check whether getAttribute exist because textNode may be an actual
   // TextNode, which does not have getAttribute.
-  if (textNode && textNode.getAttribute &&
-      textNode.getAttribute('_moz_editor_bogus_node')) {
+
+  var hasBogusNode = textNode && textNode.getAttribute &&
+                     textNode.getAttribute('_moz_editor_bogus_node');
+  if (hasBogusNode) {
     // At least in FF >= 6, assigning '' to innerHTML of a contentEditable
     // element will results in textNode being modified into:
     // <br _moz_editor_bogus_node="TRUE" _moz_dirty=""> instead of nulling
@@ -1079,8 +1120,13 @@ function doTestPlaceCursorAtEnd(opt_html, opt_parentId, opt_offset) {
   var offset = goog.isDefAndNotNull(opt_offset) ?
       opt_offset :
       textNode ? endNode.nodeValue.length : endNode.childNodes.length - 1;
-  assertEquals('The range should end at the ending of the node',
-      offset, range.getEndOffset());
+  if (hasBogusNode) {
+    assertEquals('The range should end at the ending of the bogus node ' +
+                 'added by FF', offset + 1, range.getEndOffset());
+  } else {
+    assertEquals('The range should end at the ending of the node',
+        offset, range.getEndOffset());
+  }
 }
 
 

closure/goog/editor/plugins/abstractdialogplugin.js

@@ -240,11 +240,8 @@ goog.editor.plugins.AbstractDialogPlugin.prototype.handleAfterHide = function(
  */
 goog.editor.plugins.AbstractDialogPlugin.prototype.restoreOriginalSelection =
     function() {
-  if (this.savedRange_) {
-    this.savedRange_.restore();
-    this.savedRange_ = null;
-  }
-  this.getFieldObject().focus();
+  this.getFieldObject().restoreSavedRange(this.savedRange_);
+  this.savedRange_ = null;
 };
 
 

closure/goog/editor/range.js

@@ -187,17 +187,7 @@ goog.editor.range.placeCursorNextTo = function(node, toLeft) {
   var offset = goog.array.indexOf(parent.childNodes, node) +
       (toLeft ? 0 : 1);
   var point = goog.editor.range.Point.createDeepestPoint(
-      parent, offset, toLeft);
-  // NOTE: It's for fixing bug that selecting HR tag breaks
-  // the cursor position In IE9. See http://b/6040468.
-  if (goog.userAgent.IE && goog.userAgent.isVersion('9') &&
-      point.node.nodeType == goog.dom.NodeType.ELEMENT &&
-      point.node.tagName == goog.dom.TagName.HR) {
-    var hr = point.node;
-    point.node = hr.parentNode;
-    point.offset = goog.array.indexOf(point.node.childNodes, hr) +
-        (toLeft ? 0 : 1);
-  }
+      parent, offset, toLeft, true);
   var range = goog.dom.Range.createCaret(point.node, point.offset);
   range.select();
   return range;
@@ -513,25 +503,39 @@ goog.editor.range.Point.prototype.getParentPoint = function() {
  *     By default, we trend rightward. If this parameter is true, then we
  *     trend leftward. The tendency to fall rightward by default is for
  *     consistency with other range APIs (like placeCursorNextTo).
+ * @param {boolean=} opt_stopOnChildlessElement If true, and we encounter
+ *     a Node which is an Element that cannot have children, we return a Point
+ *     based on its parent rather than that Node itself.
  * @return {goog.editor.range.Point} A new point.
  */
 goog.editor.range.Point.createDeepestPoint =
-    function(node, offset, opt_trendLeft) {
+    function(node, offset, opt_trendLeft, opt_stopOnChildlessElement) {
   while (node.nodeType == goog.dom.NodeType.ELEMENT) {
     var child = node.childNodes[offset];
     if (!child && !node.lastChild) {
       break;
-    }
-    if (child) {
+    } else if (child) {
       var prevSibling = child.previousSibling;
       if (opt_trendLeft && prevSibling) {
+        if (opt_stopOnChildlessElement &&
+            goog.editor.range.Point.isTerminalElement_(prevSibling)) {
+          break;
+        }
         node = prevSibling;
         offset = goog.editor.node.getLength(node);
       } else {
+        if (opt_stopOnChildlessElement &&
+            goog.editor.range.Point.isTerminalElement_(child)) {
+          break;
+        }
         node = child;
         offset = 0;
       }
     } else {
+      if (opt_stopOnChildlessElement &&
+          goog.editor.range.Point.isTerminalElement_(node.lastChild)) {
+        break;
+      }
       node = node.lastChild;
       offset = goog.editor.node.getLength(node);
     }
@@ -541,6 +545,21 @@ goog.editor.range.Point.createDeepestPoint =
 };
 
 
+/**
+ * Return true if the specified node is an Element that is not expected to have
+ * children. The createDeepestPoint() method should not traverse into
+ * such elements.
+ * @param {Node} node .
+ * @return {boolean} True if the node is an Element that does not contain
+ *     child nodes (e.g. BR, IMG).
+ * @private
+ */
+goog.editor.range.Point.isTerminalElement_ = function(node) {
+  return (node.nodeType == goog.dom.NodeType.ELEMENT &&
+          !goog.dom.canHaveChildren(node));
+};
+
+
 /**
  * Construct a point at the very end of the given node.
  * @param {Node} node The node to create a point for.

closure/goog/editor/range_test.html

@@ -379,13 +379,9 @@
   assertEquals(2, children.length);
   var node = children[0];
   var range = goog.editor.range.placeCursorNextTo(node, true);
-  if (goog.userAgent.IE && goog.userAgent.isVersion('9')) {
-    assertEquals(div, range.getStartNode());
-    assertEquals(0, range.getStartOffset());
-  } else {
-    assertEquals(children[0], range.getStartNode());
-    assertEquals(0, range.getStartOffset());
-  }
+
+  assertEquals(div, range.getStartNode());
+  assertEquals(0, range.getStartOffset());
 }
 
 function testPlaceCursorNextTo_rightOfHr() {
@@ -395,13 +391,72 @@
   assertEquals(2, children.length);
   var node = children[1];
   var range = goog.editor.range.placeCursorNextTo(node, false);
-  if (goog.userAgent.IE && goog.userAgent.isVersion('9')) {
-    assertEquals(div, range.getStartNode());
-    assertEquals(1, range.getStartOffset());
-  } else {
-    assertEquals(children[1], range.getStartNode());
-    assertEquals(0, range.getStartOffset());
-  }
+
+  assertEquals(div, range.getStartNode());
+  assertEquals(2, range.getStartOffset());
+}
+
+function testPlaceCursorNextTo_rightOfImg() {
+  var div = $('parentNode');
+  div.innerHTML =
+      'aaa<img src="https://www.google.com/images/srpr/logo3w.png">bbb';
+  var children = div.childNodes;
+  assertEquals(3, children.length);
+  var imgNode = children[1];
+  var range = goog.editor.range.placeCursorNextTo(imgNode, false);
+
+  assertEquals('range node should be the right sibling of img tag',
+      children[2], range.getStartNode());
+  assertEquals(0, range.getStartOffset());
+
+}
+
+function testPlaceCursorNextTo_rightOfImgAtEnd() {
+  var div = $('parentNode');
+  div.innerHTML =
+      'aaa<img src="https://www.google.com/images/srpr/logo3w.png">';
+  var children = div.childNodes;
+  assertEquals(2, children.length);
+  var imgNode = children[1];
+  var range = goog.editor.range.placeCursorNextTo(imgNode, false);
+
+  assertEquals('range node should be the parent of img',
+      div, range.getStartNode());
+  assertEquals('offset should be right after the img tag',
+      2, range.getStartOffset());
+
+}
+
+function testPlaceCursorNextTo_leftOfImg() {
+  var div = $('parentNode');
+  div.innerHTML =
+      '<img src="https://www.google.com/images/srpr/logo3w.png">xxx';
+  var children = div.childNodes;
+  assertEquals(2, children.length);
+  var imgNode = children[0];
+  var range = goog.editor.range.placeCursorNextTo(imgNode, true);
+
+  assertEquals('range node should be the parent of img',
+      div, range.getStartNode());
+  assertEquals('offset should point to the img tag',
+      0, range.getStartOffset());
+}
+
+function testPlaceCursorNextTo_rightOfFirstOfTwoImgTags() {
+  var div = $('parentNode');
+  div.innerHTML =
+      'aaa<img src="https://www.google.com/images/srpr/logo3w.png">' +
+      '<img src="https://www.google.com/images/srpr/logo3w.png">';
+  var children = div.childNodes;
+  assertEquals(3, children.length);
+  var imgNode = children[1];  // First of two IMG nodes
+  var range = goog.editor.range.placeCursorNextTo(imgNode, false);
+
+  assertEquals('range node should be the parent of img instead of ' +
+      'node with innerHTML=' + range.getStartNode().innerHTML,
+      div, range.getStartNode());
+  assertEquals('offset should be right after the img tag',
+      2, range.getStartOffset());
 }
 
 function testGetDeepEndPoint() {

closure/goog/testing/editor/fieldmock.js

@@ -24,6 +24,7 @@ goog.require('goog.dom');
 goog.require('goog.dom.Range');
 goog.require('goog.editor.Field');
 goog.require('goog.testing.LooseMock');
+goog.require('goog.testing.mockmatchers');
 
 
 
@@ -68,6 +69,14 @@ goog.testing.editor.FieldMock =
   this.$anyTimes();
   this.$returns(0);
 
+  this.restoreSavedRange(goog.testing.mockmatchers.ignoreArgument);
+  this.$anyTimes();
+  this.$does(function(range) {
+    if (range) {
+      range.restore();
+    }
+    this.focus();
+  });
 
   // These methods cannot be set on the prototype, because the prototype
   // gets stepped on by the mock framework.