Commit 140053e8 authored by justing's avatar justing

Reviewed by harrison

        <rdar://problem/4125131> REGRESSION (Mail): after certain steps,
        extra blank line appears when typing past end of reply-quoted text
        <rdar://problem/4024996> Applying block styles can cause assertion
        failure in inline style removal

        Prevent VisiblePositions at [br, 1] at the end of root editable elements.

        Layout tests added:
        * inserting/insert-at-end-01.html
        * inserting/insert-at-end-02.html

        Layout tests changed (fixed):
        * editing/deleting/delete-br-011.html
        * editing/deleting/delete-at-paragraph-boundaries-011.html
        * editing/inserting/insert-3786362-fix.html

        * khtml/editing/apply_style_command.cpp:
        (khtml::ApplyStyleCommand::applyInlineStyle):
        Do the layout before calculating start/end positions, not after,
        since upstream/downstream need to know who is rendered/unrendered.
        Don't do equivalentRangeCompliantPosition() on the start position
        in addition to downstream(), since it a) is confusing, b) frequently
        causes start/end to be equal, making removeInlineStyle a no-op and
        c) causes an assertion to fire.
        Only reset start/end using endingSelection() if splitTextElement was
        needed, since reseting start/end negates the work done above to swap
        start/end if they are backwards.
        When the start position points off the end of a node, that node should
        be skipped in all cases, not just the start.node() != end.node() case.

        * khtml/editing/composite_edit_command.cpp:
        (khtml::CompositeEditCommand::appendBlockPlaceholder):
        (khtml::CompositeEditCommand::insertBlockPlaceholder):
        Placeholders should be allowed in nodes that aren't blockFlow, for example,
        deleting the b in <div><span>b</span></div> should insert a placeholder.
        The assertion here should really be something like isBlockFlow ||
        isInlineFlow, but I can't assert those until deletion improves (4244964).

        * khtml/editing/delete_selection_command.cpp:
        (khtml::DeleteSelectionCommand::calculateTypingStyleAfterDelete):
        Don't try to select the placeholder when applying style to it.  It
        isn't necessary, and it's now impossible to do at the end of the
        document in any case.

        * khtml/editing/visible_position.cpp:
        (khtml::VisiblePosition::initDownstream):
        Don't create VisiblePositions at [br, 1] at the end of editable blocks, it
        produces strange/inconsistent editing behavior at the end of the document.

        * khtml/khtml_part.cpp: Fixed a comment.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@11086 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent d7e46e65
2005-11-07 Justin Garcia <justin.garcia@apple.com>
Reviewed by harrison
<rdar://problem/4125131> REGRESSION (Mail): after certain steps,
extra blank line appears when typing past end of reply-quoted text
<rdar://problem/4024996> Applying block styles can cause assertion
failure in inline style removal
Prevent VisiblePositions at [br, 1] at the end of root editable elements.
Layout tests added:
* inserting/insert-at-end-01.html
* inserting/insert-at-end-02.html
Layout tests changed (fixed):
* editing/deleting/delete-br-011.html
* editing/deleting/delete-at-paragraph-boundaries-011.html
* editing/inserting/insert-3786362-fix.html
* khtml/editing/apply_style_command.cpp:
(khtml::ApplyStyleCommand::applyInlineStyle):
Do the layout before calculating start/end positions, not after,
since upstream/downstream need to know who is rendered/unrendered.
Don't do equivalentRangeCompliantPosition() on the start position
in addition to downstream(), since it a) is confusing, b) frequently
causes start/end to be equal, making removeInlineStyle a no-op and
c) causes an assertion to fire.
Only reset start/end using endingSelection() if splitTextElement was
needed, since reseting start/end negates the work done above to swap
start/end if they are backwards.
When the start position points off the end of a node, that node should
be skipped in all cases, not just the start.node() != end.node() case.
* khtml/editing/composite_edit_command.cpp:
(khtml::CompositeEditCommand::appendBlockPlaceholder):
(khtml::CompositeEditCommand::insertBlockPlaceholder):
Placeholders should be allowed in nodes that aren't blockFlow, for example,
deleting the b in <div><span>b</span></div> should insert a placeholder.
The assertion here should really be something like isBlockFlow ||
isInlineFlow, but I can't assert those until deletion improves (4244964).
* khtml/editing/delete_selection_command.cpp:
(khtml::DeleteSelectionCommand::calculateTypingStyleAfterDelete):
Don't try to select the placeholder when applying style to it. It
isn't necessary, and it's now impossible to do at the end of the
document in any case.
* khtml/editing/visible_position.cpp:
(khtml::VisiblePosition::initDownstream):
Don't create VisiblePositions at [br, 1] at the end of editable blocks, it
produces strange/inconsistent editing behavior at the end of the document.
* khtml/khtml_part.cpp: Fixed a comment.
2005-11-06 Anders Carlsson <andersca@mac.com>
Reviewed by Darin.
......@@ -501,8 +501,13 @@ void ApplyStyleCommand::applyRelativeFontStyleChange(CSSMutableStyleDeclarationI
void ApplyStyleCommand::applyInlineStyle(CSSMutableStyleDeclarationImpl *style)
{
// update document layout once before removing styles
// so that we avoid the expense of updating before each and every call
// to check a computed style
document()->updateLayout();
// adjust to the positions we want to use for applying style
Position start(endingSelection().start().downstream().equivalentRangeCompliantPosition());
Position start(endingSelection().start().downstream());
Position end(endingSelection().end().upstream());
if (RangeImpl::compareBoundaryPoints(end, start) < 0) {
......@@ -511,11 +516,6 @@ void ApplyStyleCommand::applyInlineStyle(CSSMutableStyleDeclarationImpl *style)
end = swap;
}
// update document layout once before removing styles
// so that we avoid the expense of updating before each and every call
// to check a computed style
document()->updateLayout();
// split the start node and containing element if the selection starts inside of it
bool splitStart = splitTextElementAtStartIfNeeded(start, end);
if (splitStart) {
......@@ -525,8 +525,10 @@ void ApplyStyleCommand::applyInlineStyle(CSSMutableStyleDeclarationImpl *style)
// split the end node and containing element if the selection ends inside of it
bool splitEnd = splitTextElementAtEndIfNeeded(start, end);
start = endingSelection().start();
end = endingSelection().end();
if (splitEnd) {
start = endingSelection().start();
end = endingSelection().end();
}
// Remove style from the selection.
// Use the upstream position of the start for removing style.
......@@ -556,14 +558,13 @@ void ApplyStyleCommand::applyInlineStyle(CSSMutableStyleDeclarationImpl *style)
// to check a computed style
document()->updateLayout();
NodeImpl *node = start.node();
if (start.offset() >= start.node()->caretMaxOffset())
node = node->traverseNextNode();
if (start.node() == end.node()) {
// simple case...start and end are the same node
addInlineStyleIfNeeded(style, start.node(), end.node());
}
else {
NodeImpl *node = start.node();
if (start.offset() >= start.node()->caretMaxOffset())
node = node->traverseNextNode();
addInlineStyleIfNeeded(style, node, node);
} else {
while (1) {
if (node->childNodeCount() == 0 && node->renderer() && node->renderer()->isInline()) {
NodeImpl *runStart = node;
......
......@@ -438,8 +438,9 @@ NodeImpl *CompositeEditCommand::appendBlockPlaceholder(NodeImpl *node)
{
if (!node)
return NULL;
ASSERT(node->renderer() && node->renderer()->isBlockFlow());
// Should assert isBlockFlow || isInlineFlow when deletion improves. See 4244964.
ASSERT(node->renderer());
NodeImpl *placeholder = createBlockPlaceholderElement(document());
appendNode(placeholder, node);
......@@ -451,7 +452,8 @@ NodeImpl *CompositeEditCommand::insertBlockPlaceholder(const Position &pos)
if (pos.isNull())
return NULL;
ASSERT(pos.node()->renderer() && pos.node()->renderer()->isBlockFlow());
// Should assert isBlockFlow || isInlineFlow when deletion improves. See 4244964.
ASSERT(pos.node()->renderer());
NodeImpl *placeholder = createBlockPlaceholderElement(document());
insertNodeAt(placeholder, pos.node(), pos.offset());
......
......@@ -666,13 +666,7 @@ void DeleteSelectionCommand::calculateTypingStyleAfterDelete(NodeImpl *insertedP
// then start typing. In this case, the typing style is applied right now, and
// is not retained until the next typing action.
// FIXME: is this even right? I don't think post-deletion typing style is supposed
// to be saved across clicking away and clicking back, it certainly isn't in TextEdit
Position pastPlaceholder(insertedPlaceholder, 1);
setEndingSelection(SelectionController(m_endingPosition, m_selectionToDelete.endAffinity(), pastPlaceholder, DOWNSTREAM));
setEndingSelection(SelectionController(Position(insertedPlaceholder, 0), DOWNSTREAM));
applyStyle(m_typingStyle, EditActionUnspecified);
m_typingStyle->deref();
......
......@@ -147,7 +147,7 @@ void VisiblePosition::initDownstream(const Position &pos)
if (next.isNotNull()) {
m_deepPosition = next;
} else if (isRenderedBR(deepPos.node()) && deepPos.offset() == 1) {
m_deepPosition = deepPos;
m_deepPosition = Position(deepPos.node(), 0);
} else {
Position previous = previousVisiblePosition(deepPos);
if (previous.isNotNull())
......
......@@ -5112,7 +5112,7 @@ void KHTMLPart::slotSelectAll()
static_cast<KHTMLPart *>(part)->selectAll();
}
#endif // APPLE_CHANGES
#endif // !APPLE_CHANGES
void KHTMLPart::startAutoScroll()
{
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment