Browse files

Automated g4 rollback

*** Reason for rollback ***

Rolling the original change forward

*** Original change description ***

Automated g4 rollback

*** Reason for rollback ***

I can consistently make IE8 segfault after this change

*** Original change description ***

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 c

... description truncated by g4 rollback ...

R=nicksantos
DELTA=208 (167 added, 18 deleted, 23 changed)


Revision created by MOE tool push_codebase.
MOE_MIGRATION=6098


git-svn-id: http://closure-library.googlecode.com/svn/trunk@2424 0b95b8e8-c90f-11de-9d4f-f947ee5921c8
  • Loading branch information...
1 parent a90ff85 commit bd2a9d2545634ff983f6fcb9a83dc8dad98fca83 jamisong@google.com committed Jan 8, 2013
View
2 closure/goog/deps.js
@@ -568,7 +568,7 @@ goog.addDependency('testing/continuationtestcase.js', ['goog.testing.Continuatio
goog.addDependency('testing/deferredtestcase.js', ['goog.testing.DeferredTestCase'], ['goog.async.Deferred', 'goog.testing.AsyncTestCase', 'goog.testing.TestCase']);
goog.addDependency('testing/dom.js', ['goog.testing.dom'], ['goog.dom', 'goog.dom.NodeIterator', 'goog.dom.NodeType', 'goog.dom.TagIterator', 'goog.dom.TagName', 'goog.dom.classes', 'goog.iter', 'goog.object', 'goog.string', 'goog.style', 'goog.testing.asserts', 'goog.userAgent']);
goog.addDependency('testing/editor/dom.js', ['goog.testing.editor.dom'], ['goog.dom.NodeType', 'goog.dom.TagIterator', 'goog.dom.TagWalkType', 'goog.iter', 'goog.string', 'goog.testing.asserts']);
-goog.addDependency('testing/editor/fieldmock.js', ['goog.testing.editor.FieldMock'], ['goog.dom', 'goog.dom.Range', 'goog.editor.Field', 'goog.testing.LooseMock']);
+goog.addDependency('testing/editor/fieldmock.js', ['goog.testing.editor.FieldMock'], ['goog.dom', 'goog.dom.Range', 'goog.editor.Field', 'goog.testing.LooseMock', 'goog.testing.mockmatchers']);
goog.addDependency('testing/editor/testhelper.js', ['goog.testing.editor.TestHelper'], ['goog.Disposable', 'goog.dom', 'goog.dom.Range', 'goog.editor.BrowserFeature', 'goog.editor.node', 'goog.testing.dom']);
goog.addDependency('testing/events/eventobserver.js', ['goog.testing.events.EventObserver'], ['goog.array']);
goog.addDependency('testing/events/events.js', ['goog.testing.events', 'goog.testing.events.Event'], ['goog.events', 'goog.events.BrowserEvent', 'goog.events.BrowserEvent.MouseButton', 'goog.events.BrowserFeature', 'goog.events.EventType', 'goog.events.KeyCodes', 'goog.object', 'goog.style', 'goog.userAgent']);
View
18 closure/goog/editor/field.js
@@ -2331,6 +2331,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.
*
* @param {string=} opt_iframeSrc URL to set the iframe src to if necessary.
View
54 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());
+ }
}
View
7 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;
};
View
47 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);
}
@@ -542,6 +546,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.
* @return {goog.editor.range.Point} A new point.
View
83 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() {
View
9 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.

0 comments on commit bd2a9d2

Please sign in to comment.