Reviewed by darin
Fixed (was missing a newline):
* editing/execCommand/
4917055-expected.txt:
Tests setting a caet before the image and changing its alignment:
* editing/execCommand/
5080333-1-expected.checksum: Added.
* editing/execCommand/
5080333-1-expected.png: Added.
* editing/execCommand/
5080333-1-expected.txt: Added.
* editing/execCommand/
5080333-1.html: Added.
Tests selecting the image and changing its alignment:
* editing/execCommand/
5080333-2-expected.checksum: Added.
* editing/execCommand/
5080333-2-expected.png: Added.
* editing/execCommand/
5080333-2-expected.txt: Added.
* editing/execCommand/
5080333-2.html: Added.
WebCore:
Reviewed by darin
<rdar://problem/
5080333>
REGRESSION: Selection changes when changing the alignment of an image
Regression occurred when we started using moveParagraphs
to move content in applyBlockStyle. moveParagraphs
moves by copying, deleting and reinserting content, and
so must be accompanied by selection preservation code.
That code uses rangeFromLocationAndLength and rangeLength,
which use TextIterators, which don't emit anything for images
and other replaced elements, causing this bug.
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::applyBlockStyle): Ask rangeLength
and rangeFromLocationAndLength to request that their
TextIterators emit spaces for replaced elements.
Use rangeCompliantEquivalent()s when creating a Range from
VisiblePositions, since some VisiblePositions have illegal
deepEquivalent()s.
* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::moveParagraphs): Ditto.
* editing/TextIterator.cpp:
(WebCore::TextIterator::TextIterator):
(WebCore::TextIterator::handleReplacedElement): Emit
a space if requested.
(WebCore::TextIterator::representNodeOffsetZero): Emit
ranges before m_node, not around m_lastTextNode. These
ranges should represent the part of the document associated
with the emitted character.
(WebCore::TextIterator::rangeLength): Take in the new bool.
(WebCore::TextIterator::rangeFromLocationAndLength): Ditto.
Also, don't loop an extra time after finding the end of the
range when we're looking for zero length ranges. This appeared
to be a workaround for the bugs fixed in representNodeOffsetZero
in this patch.
* editing/TextIterator.h:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@20449
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2007-03-23 Justin Garcia <justin.garcia@apple.com>
+
+ Reviewed by darin
+
+ Fixed (was missing a newline):
+ * editing/execCommand/4917055-expected.txt:
+ Tests setting a caet before the image and changing its alignment:
+ * editing/execCommand/5080333-1-expected.checksum: Added.
+ * editing/execCommand/5080333-1-expected.png: Added.
+ * editing/execCommand/5080333-1-expected.txt: Added.
+ * editing/execCommand/5080333-1.html: Added.
+ Tests selecting the image and changing its alignment:
+ * editing/execCommand/5080333-2-expected.checksum: Added.
+ * editing/execCommand/5080333-2-expected.png: Added.
+ * editing/execCommand/5080333-2-expected.txt: Added.
+ * editing/execCommand/5080333-2.html: Added.
+
2007-03-23 Adele Peterson <adele@apple.com>
Reviewed by Darin.
--- /dev/null
+76386b683b85d7193166d8a0022f696a
\ No newline at end of file
--- /dev/null
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+ RenderBlock {HTML} at (0,0) size 800x600
+ RenderBody {BODY} at (8,8) size 784x584
+ RenderBlock {P} at (0,0) size 784x54
+ RenderText {#text} at (0,0) size 758x54
+ text run at (0,0) width 737: "This tests for a bug where changing the alignment of an image would result in a selection that wasn't the one that was"
+ text run at (0,18) width 235: "present before the alignment change. "
+ text run at (235,18) width 523: "The image should be centered and the caret should be the same before and after the"
+ text run at (0,36) width 63: "operation."
+ RenderBlock {DIV} at (0,70) size 784x139
+ RenderBlock (anonymous) at (0,0) size 784x18
+ RenderText {#text} at (0,0) size 21x18
+ text run at (0,0) width 21: "foo"
+ RenderBR {BR} at (21,14) size 0x0
+ RenderBlock {DIV} at (0,18) size 784x103
+ RenderImage {IMG} at (354,0) size 76x103
+ RenderBlock (anonymous) at (0,121) size 784x18
+ RenderText {#text} at (0,0) size 22x18
+ text run at (0,0) width 22: "baz"
+caret: position 0 of child 0 {IMG} of child 2 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
--- /dev/null
+<p>This tests for a bug where changing the alignment of an image would result in a selection that wasn't the one that was present before the alignment change. The image should be centered and the caret should be the same before and after the operation.</p>
+<div id="div" contenteditable="true">foo<br><img src="../resources/abe.jpg"><br>baz</div>
+
+<script>
+var div = document.getElementById("div");
+var sel = window.getSelection();
+
+sel.setPosition(div, 0);
+sel.modify("move", "forward", "paragraphBoundary");
+sel.modify("move", "forward", "character");
+
+document.execCommand("JustifyCenter");
+</script>
--- /dev/null
+26e7fb895fc46b6efcc6eb14a9da7034
\ No newline at end of file
--- /dev/null
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+ RenderBlock {HTML} at (0,0) size 800x600
+ RenderBody {BODY} at (8,8) size 784x584
+ RenderBlock {P} at (0,0) size 784x54
+ RenderText {#text} at (0,0) size 783x54
+ text run at (0,0) width 737: "This tests for a bug where changing the alignment of an image would result in a selection that wasn't the one that was"
+ text run at (0,18) width 235: "present before the alignment change. "
+ text run at (235,18) width 548: "The image should be centered and the selection should be the same before and after the"
+ text run at (0,36) width 63: "operation."
+ RenderBlock {DIV} at (0,70) size 784x139
+ RenderBlock (anonymous) at (0,0) size 784x18
+ RenderText {#text} at (0,0) size 21x18
+ text run at (0,0) width 21: "foo"
+ RenderBR {BR} at (21,14) size 0x0
+ RenderBlock {DIV} at (0,18) size 784x103
+ RenderImage {IMG} at (354,0) size 76x103
+ RenderBlock (anonymous) at (0,121) size 784x18
+ RenderText {#text} at (0,0) size 22x18
+ text run at (0,0) width 22: "baz"
+selection start: position 0 of child 0 {IMG} of child 2 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+selection end: position 1 of child 0 {IMG} of child 2 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
--- /dev/null
+<p>This tests for a bug where changing the alignment of an image would result in a selection that wasn't the one that was present before the alignment change. The image should be centered and the selection should be the same before and after the operation.</p>
+<div id="div" contenteditable="true">foo<br><img src="../resources/abe.jpg"><br>baz</div>
+
+<script>
+var div = document.getElementById("div");
+var sel = window.getSelection();
+
+sel.setPosition(div, 0);
+sel.modify("move", "forward", "paragraphBoundary");
+sel.modify("move", "forward", "character");
+sel.modify("extend", "forward", "character");
+
+document.execCommand("JustifyCenter");
+</script>
+2007-03-23 Justin Garcia <justin.garcia@apple.com>
+
+ Reviewed by darin
+
+ <rdar://problem/5080333>
+ REGRESSION: Selection changes when changing the alignment of an image
+
+ Regression occurred when we started using moveParagraphs
+ to move content in applyBlockStyle. moveParagraphs
+ moves by copying, deleting and reinserting content, and
+ so must be accompanied by selection preservation code.
+ That code uses rangeFromLocationAndLength and rangeLength,
+ which use TextIterators, which don't emit anything for images
+ and other replaced elements, causing this bug.
+
+ * editing/ApplyStyleCommand.cpp:
+ (WebCore::ApplyStyleCommand::applyBlockStyle): Ask rangeLength
+ and rangeFromLocationAndLength to request that their
+ TextIterators emit spaces for replaced elements.
+ Use rangeCompliantEquivalent()s when creating a Range from
+ VisiblePositions, since some VisiblePositions have illegal
+ deepEquivalent()s.
+ * editing/CompositeEditCommand.cpp:
+ (WebCore::CompositeEditCommand::moveParagraphs): Ditto.
+ * editing/TextIterator.cpp:
+ (WebCore::TextIterator::TextIterator):
+ (WebCore::TextIterator::handleReplacedElement): Emit
+ a space if requested.
+ (WebCore::TextIterator::representNodeOffsetZero): Emit
+ ranges before m_node, not around m_lastTextNode. These
+ ranges should represent the part of the document associated
+ with the emitted character.
+ (WebCore::TextIterator::rangeLength): Take in the new bool.
+ (WebCore::TextIterator::rangeFromLocationAndLength): Ditto.
+ Also, don't loop an extra time after finding the end of the
+ range when we're looking for zero length ranges. This appeared
+ to be a workaround for the bugs fixed in representNodeOffsetZero
+ in this patch.
+ * editing/TextIterator.h:
+
2007-03-24 Mark Rowe <mrowe@apple.com>
Rubber-stamped by Darin.
// Calculate start and end indices from the start of the tree that they're in.
Node* scope = highestAncestor(visibleStart.deepEquivalent().node());
Position rangeStart(scope, 0);
- RefPtr<Range> startRange = new Range(document(), rangeStart, visibleStart.deepEquivalent());
- RefPtr<Range> endRange = new Range(document(), rangeStart, visibleEnd.deepEquivalent());
- int startIndex = TextIterator::rangeLength(startRange.get());
- int endIndex = TextIterator::rangeLength(endRange.get());
+ RefPtr<Range> startRange = new Range(document(), rangeStart, rangeCompliantEquivalent(visibleStart.deepEquivalent()));
+ RefPtr<Range> endRange = new Range(document(), rangeStart, rangeCompliantEquivalent(visibleEnd.deepEquivalent()));
+ int startIndex = TextIterator::rangeLength(startRange.get(), true);
+ int endIndex = TextIterator::rangeLength(endRange.get(), true);
VisiblePosition paragraphStart(startOfParagraph(visibleStart));
VisiblePosition nextParagraphStart(endOfParagraph(paragraphStart).next());
nextParagraphStart = endOfParagraph(paragraphStart).next();
}
- updateStartEnd(TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), startIndex, 0)->startPosition(),
- TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), endIndex, 0)->startPosition());
+ updateStartEnd(TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), startIndex, 0, true)->startPosition(),
+ TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), endIndex, 0, true)->startPosition());
}
#define NoFontDelta (0.0f)
startIndex = 0;
if (startInParagraph) {
RefPtr<Range> startRange = new Range(document(), startOfParagraphToMove.deepEquivalent(), visibleStart.deepEquivalent());
- startIndex = TextIterator::rangeLength(startRange.get());
+ startIndex = TextIterator::rangeLength(startRange.get(), true);
}
endIndex = 0;
if (endInParagraph) {
RefPtr<Range> endRange = new Range(document(), startOfParagraphToMove.deepEquivalent(), visibleEnd.deepEquivalent());
- endIndex = TextIterator::rangeLength(endRange.get());
+ endIndex = TextIterator::rangeLength(endRange.get(), true);
}
}
}
}
RefPtr<Range> startToDestinationRange(new Range(document(), Position(document(), 0), rangeCompliantEquivalent(destination.deepEquivalent())));
- destinationIndex = TextIterator::rangeLength(startToDestinationRange.get());
+ destinationIndex = TextIterator::rangeLength(startToDestinationRange.get(), true);
setEndingSelection(destination);
applyCommandToComposite(new ReplaceSelectionCommand(document(), fragment.get(), true, false, !preserveStyle, false));
if (preserveSelection && startIndex != -1) {
- setEndingSelection(Selection(TextIterator::rangeFromLocationAndLength(document()->documentElement(), destinationIndex + startIndex, 0)->startPosition(),
- TextIterator::rangeFromLocationAndLength(document()->documentElement(), destinationIndex + endIndex, 0)->startPosition(),
+ setEndingSelection(Selection(TextIterator::rangeFromLocationAndLength(document()->documentElement(), destinationIndex + startIndex, 0, true)->startPosition(),
+ TextIterator::rangeFromLocationAndLength(document()->documentElement(), destinationIndex + endIndex, 0, true)->startPosition(),
DOWNSTREAM));
}
}
{
}
-TextIterator::TextIterator(const Range *r) : m_startContainer(0), m_startOffset(0), m_endContainer(0), m_endOffset(0), m_positionNode(0)
+TextIterator::TextIterator(const Range* r, bool emitSpaceForReplacedElements)
+ : m_startContainer(0)
+ , m_startOffset(0)
+ , m_endContainer(0)
+ , m_endOffset(0)
+ , m_positionNode(0)
+ , m_emitSpaceForReplacedElements(emitSpaceForReplacedElements)
{
if (!r)
return;
m_haveEmitted = true;
+ if (m_emitSpaceForReplacedElements) {
+ emitCharacter(' ', m_node->parentNode(), m_node, 0, 1);
+ return true;
+ }
+
m_positionNode = m_node->parentNode();
m_positionOffsetBaseNode = m_node;
m_positionStartOffset = 0;
return;
if (shouldEmitTabBeforeNode(m_node))
- emitCharacter('\t', m_lastTextNode->parentNode(), m_lastTextNode, 0, 1);
+ emitCharacter('\t', m_node->parentNode(), m_node, 0, 0);
else if (shouldEmitNewlineBeforeNode(m_node))
- emitCharacter('\n', m_lastTextNode->parentNode(), m_lastTextNode, 0, 1);
+ emitCharacter('\n', m_node->parentNode(), m_node, 0, 0);
else if (shouldEmitSpaceBeforeAndAfterNode(m_node))
- emitCharacter(' ', m_lastTextNode->parentNode(), m_lastTextNode, 0, 1);
+ emitCharacter(' ', m_node->parentNode(), m_node, 0, 0);
}
bool TextIterator::handleNonTextNode()
&& memcmp(m_buffer, m_target.characters() + tailSpace, headSpace * sizeof(UChar)) == 0;
}
-int TextIterator::rangeLength(const Range *r)
+int TextIterator::rangeLength(const Range *r, bool spacesForReplacedElements)
{
int length = 0;
- for (TextIterator it(r); !it.atEnd(); it.advance())
+ for (TextIterator it(r, spacesForReplacedElements); !it.atEnd(); it.advance())
length += it.length();
return length;
return result.release();
}
-PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(Element *scope, int rangeLocation, int rangeLength)
+PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(Element *scope, int rangeLocation, int rangeLength, bool spacesForReplacedElements)
{
RefPtr<Range> resultRange = scope->document()->createRange();
RefPtr<Range> textRunRange;
- TextIterator it(rangeOfContents(scope).get());
+ TextIterator it(rangeOfContents(scope).get(), spacesForReplacedElements);
// FIXME: the atEnd() check shouldn't be necessary, workaround for <http://bugs.webkit.org/show_bug.cgi?id=6289>.
if (rangeLocation == 0 && rangeLength == 0 && it.atEnd()) {
else
resultRange->setEnd(textRunRange->endContainer(exception), textRunRange->endOffset(exception), exception);
}
- if (!(rangeLength == 0 && rangeEnd == docTextPosition + len)) {
- docTextPosition += len;
- break;
- }
+ docTextPosition += len;
+ break;
}
docTextPosition += len;
}
{
public:
TextIterator();
- explicit TextIterator(const Range *);
+ explicit TextIterator(const Range*, bool emitSpaceForReplacedElements = false);
bool atEnd() const { return !m_positionNode; }
void advance();
PassRefPtr<Range> range() const;
- static int rangeLength(const Range *r);
- static PassRefPtr<Range> rangeFromLocationAndLength(Element *scope, int rangeLocation, int rangeLength);
+ static int rangeLength(const Range*, bool spacesForReplacedElements = false);
+ static PassRefPtr<Range> rangeFromLocationAndLength(Element* scope, int rangeLocation, int rangeLength, bool spacesForReplacedElements = false);
static PassRefPtr<Range> subrange(Range* entireRange, int characterOffset, int characterCount);
private:
// Used when deciding whether to emit a "positioning" (e.g. newline) before any other content
bool m_haveEmitted;
+
+ // Used by selection preservation code.
+ bool m_emitSpaceForReplacedElements;
};
// Iterates through the DOM range, returning all the text, and 0-length boundaries
git://git.webkit.org / WebKit-https.git / commitdiff
