[webkit-changes] cvs commit: WebCore/khtml/editing
apply_style_command.cpp composite_edit_command.cpp
delete_selection_command.cpp visible_position.cpp
Justin
justing at opensource.apple.com
Mon Nov 7 11:59:23 PST 2005
justing 05/11/07 11:59:23
Modified: . ChangeLog
khtml khtml_part.cpp
khtml/editing apply_style_command.cpp
composite_edit_command.cpp
delete_selection_command.cpp visible_position.cpp
Log:
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.
Revision Changes Path
1.340 +55 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.339
retrieving revision 1.340
diff -u -r1.339 -r1.340
--- ChangeLog 6 Nov 2005 21:47:24 -0000 1.339
+++ ChangeLog 7 Nov 2005 19:59:20 -0000 1.340
@@ -1,3 +1,58 @@
+2005-11-07 Justin Garcia <justin.garcia at 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 at mac.com>
Reviewed by Darin.
1.357 +1 -1 WebCore/khtml/khtml_part.cpp
Index: khtml_part.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/khtml_part.cpp,v
retrieving revision 1.356
retrieving revision 1.357
diff -u -r1.356 -r1.357
--- khtml_part.cpp 3 Nov 2005 19:12:03 -0000 1.356
+++ khtml_part.cpp 7 Nov 2005 19:59:21 -0000 1.357
@@ -5112,7 +5112,7 @@
static_cast<KHTMLPart *>(part)->selectAll();
}
-#endif // APPLE_CHANGES
+#endif // !APPLE_CHANGES
void KHTMLPart::startAutoScroll()
{
1.17 +16 -15 WebCore/khtml/editing/apply_style_command.cpp
Index: apply_style_command.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/apply_style_command.cpp,v
retrieving revision 1.16
retrieving revision 1.17
diff -u -r1.16 -r1.17
--- apply_style_command.cpp 3 Oct 2005 21:12:18 -0000 1.16
+++ apply_style_command.cpp 7 Nov 2005 19:59:22 -0000 1.17
@@ -501,8 +501,13 @@
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 @@
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 @@
// 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 @@
// 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;
1.16 +5 -3 WebCore/khtml/editing/composite_edit_command.cpp
Index: composite_edit_command.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/composite_edit_command.cpp,v
retrieving revision 1.15
retrieving revision 1.16
diff -u -r1.15 -r1.16
--- composite_edit_command.cpp 3 Oct 2005 21:12:18 -0000 1.15
+++ composite_edit_command.cpp 7 Nov 2005 19:59:22 -0000 1.16
@@ -438,8 +438,9 @@
{
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 @@
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());
1.21 +1 -7 WebCore/khtml/editing/delete_selection_command.cpp
Index: delete_selection_command.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/delete_selection_command.cpp,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -r1.20 -r1.21
--- delete_selection_command.cpp 8 Oct 2005 00:50:29 -0000 1.20
+++ delete_selection_command.cpp 7 Nov 2005 19:59:22 -0000 1.21
@@ -666,13 +666,7 @@
// 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();
1.61 +1 -1 WebCore/khtml/editing/visible_position.cpp
Index: visible_position.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/visible_position.cpp,v
retrieving revision 1.60
retrieving revision 1.61
diff -u -r1.60 -r1.61
--- visible_position.cpp 3 Oct 2005 21:12:21 -0000 1.60
+++ visible_position.cpp 7 Nov 2005 19:59:22 -0000 1.61
@@ -147,7 +147,7 @@
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())
More information about the webkit-changes
mailing list