Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

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 change (for IE) the order

... description truncated by g4 rollback ...

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


Revision created by MOE tool push_codebase.
MOE_MIGRATION=6064


git-svn-id: http://closure-library.googlecode.com/svn/trunk@2409 0b95b8e8-c90f-11de-9d4f-f947ee5921c8
  • Loading branch information...
commit 8b8389ddcc787ceb4b7ed1982d4bf9b9689061fd 1 parent 578422e
nicksantos@google.com authored
View
4 closure/goog/deps.js
@@ -386,7 +386,7 @@ goog.addDependency('math/math.js', ['goog.math'], ['goog.array']);
goog.addDependency('math/matrix.js', ['goog.math.Matrix'], ['goog.array', 'goog.math', 'goog.math.Size']);
goog.addDependency('math/range.js', ['goog.math.Range'], []);
goog.addDependency('math/rangeset.js', ['goog.math.RangeSet'], ['goog.array', 'goog.iter.Iterator', 'goog.iter.StopIteration', 'goog.math.Range']);
-goog.addDependency('math/rect.js', ['goog.math.Rect'], ['goog.math.Box', 'goog.math.Coordinate', 'goog.math.Size']);
+goog.addDependency('math/rect.js', ['goog.math.Rect'], ['goog.math.Box', 'goog.math.Size']);
goog.addDependency('math/size.js', ['goog.math.Size'], []);
goog.addDependency('math/tdma.js', ['goog.math.tdma'], []);
goog.addDependency('math/vec2.js', ['goog.math.Vec2'], ['goog.math', 'goog.math.Coordinate']);
@@ -565,7 +565,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.testing.mockmatchers']);
+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/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,24 +2331,6 @@ 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,45 +1014,6 @@ 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();
}
@@ -1101,10 +1062,8 @@ 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.
-
- var hasBogusNode = textNode && textNode.getAttribute &&
- textNode.getAttribute('_moz_editor_bogus_node');
- if (hasBogusNode) {
+ if (textNode && textNode.getAttribute &&
+ textNode.getAttribute('_moz_editor_bogus_node')) {
// 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
@@ -1120,13 +1079,8 @@ function doTestPlaceCursorAtEnd(opt_html, opt_parentId, opt_offset) {
var offset = goog.isDefAndNotNull(opt_offset) ?
opt_offset :
textNode ? endNode.nodeValue.length : endNode.childNodes.length - 1;
- 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());
- }
+ assertEquals('The range should end at the ending of the node',
+ offset, range.getEndOffset());
}
View
7 closure/goog/editor/plugins/abstractdialogplugin.js
@@ -240,8 +240,11 @@ goog.editor.plugins.AbstractDialogPlugin.prototype.handleAfterHide = function(
*/
goog.editor.plugins.AbstractDialogPlugin.prototype.restoreOriginalSelection =
function() {
- this.getFieldObject().restoreSavedRange(this.savedRange_);
- this.savedRange_ = null;
+ if (this.savedRange_) {
+ this.savedRange_.restore();
+ this.savedRange_ = null;
+ }
+ this.getFieldObject().focus();
};
View
47 closure/goog/editor/range.js
@@ -187,7 +187,17 @@ 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, true);
+ 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);
+ }
var range = goog.dom.Range.createCaret(point.node, point.offset);
range.select();
return range;
@@ -503,39 +513,25 @@ 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, opt_stopOnChildlessElement) {
+ function(node, offset, opt_trendLeft) {
while (node.nodeType == goog.dom.NodeType.ELEMENT) {
var child = node.childNodes[offset];
if (!child && !node.lastChild) {
break;
- } else if (child) {
+ }
+ 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);
}
@@ -546,21 +542,6 @@ 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,9 +379,13 @@
assertEquals(2, children.length);
var node = children[0];
var range = goog.editor.range.placeCursorNextTo(node, true);
-
- assertEquals(div, range.getStartNode());
- assertEquals(0, range.getStartOffset());
+ 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());
+ }
}
function testPlaceCursorNextTo_rightOfHr() {
@@ -391,72 +395,13 @@
assertEquals(2, children.length);
var node = children[1];
var range = goog.editor.range.placeCursorNextTo(node, false);
-
- 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());
+ 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());
+ }
}
function testGetDeepEndPoint() {
View
9 closure/goog/testing/editor/fieldmock.js
@@ -24,7 +24,6 @@ goog.require('goog.dom');
goog.require('goog.dom.Range');
goog.require('goog.editor.Field');
goog.require('goog.testing.LooseMock');
-goog.require('goog.testing.mockmatchers');
@@ -69,14 +68,6 @@ 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.
Please sign in to comment.
Something went wrong with that request. Please try again.