[webkit-changes] cvs commit: WebCore/khtml/editing
SelectionController.cpp SelectionController.h
Justin
justing at opensource.apple.com
Mon Dec 5 20:29:25 PST 2005
justing 05/12/05 20:29:24
Modified: . ChangeLog
khtml khtml_part.cpp
khtml/editing SelectionController.cpp SelectionController.h
Log:
<http://bugzilla.opendarwin.org/show_bug.cgi?id=5936>
REGRESSION: selection keeps growing after double-click
Also filed as <rdar://problem/4364425>
Reviewed by darin, harrison
Rolled back previous change, simplified expansion, fixed
adjustForEditableContent to work in cases where start/end are
distinct from base/extent, renamed m_baseIsStart to m_baseIsFirst.
Added two new layout tests in editing/selection to test
modifying selections created with double and triple clicks.
* khtml/editing/SelectionController.cpp:
(khtml::SelectionController::SelectionController):
(khtml::SelectionController::init):
(khtml::SelectionController::operator=):
(khtml::SelectionController::expandUsingGranularity):
(khtml::SelectionController::adjustForEditableContent):
(khtml::SelectionController::validate):
* khtml/editing/SelectionController.h:
* khtml/khtml_part.cpp:
(KHTMLPart::handleMouseMoveEventSelection):
(KHTMLPart::khtmlMouseMoveEvent):
Revision Changes Path
1.481 +27 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.480
retrieving revision 1.481
diff -u -r1.480 -r1.481
--- ChangeLog 6 Dec 2005 02:41:41 -0000 1.480
+++ ChangeLog 6 Dec 2005 04:29:18 -0000 1.481
@@ -1,3 +1,30 @@
+2005-12-05 Justin Garcia <justin.garcia at apple.com>
+
+ <http://bugzilla.opendarwin.org/show_bug.cgi?id=5936>
+ REGRESSION: selection keeps growing after double-click
+ Also filed as <rdar://problem/4364425>
+
+ Reviewed by darin, harrison
+
+ Rolled back previous change, simplified expansion, fixed
+ adjustForEditableContent to work in cases where start/end are
+ distinct from base/extent, renamed m_baseIsStart to m_baseIsFirst.
+
+ Added two new layout tests in editing/selection to test
+ modifying selections created with double and triple clicks.
+
+ * khtml/editing/SelectionController.cpp:
+ (khtml::SelectionController::SelectionController):
+ (khtml::SelectionController::init):
+ (khtml::SelectionController::operator=):
+ (khtml::SelectionController::expandUsingGranularity):
+ (khtml::SelectionController::adjustForEditableContent):
+ (khtml::SelectionController::validate):
+ * khtml/editing/SelectionController.h:
+ * khtml/khtml_part.cpp:
+ (KHTMLPart::handleMouseMoveEventSelection):
+ (KHTMLPart::khtmlMouseMoveEvent):
+
2005-12-05 Eric Seidel <eseidel at apple.com>
Rubber-stamped by mjs.
1.364 +3 -5 WebCore/khtml/khtml_part.cpp
Index: khtml_part.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/khtml_part.cpp,v
retrieving revision 1.363
retrieving revision 1.364
diff -u -r1.363 -r1.364
--- khtml_part.cpp 4 Dec 2005 11:35:02 -0000 1.363
+++ khtml_part.cpp 6 Dec 2005 04:29:23 -0000 1.364
@@ -2693,26 +2693,24 @@
// done in khtmlMousePressEvent, but not if the mouse press was on an existing selection.
SelectionController sel = selection();
sel.clearModifyBias();
+
if (!d->m_beganSelectingText) {
d->m_beganSelectingText = true;
sel.moveTo(pos);
}
sel.setExtent(pos);
- if (d->m_selectionGranularity != CHARACTER) {
+ if (d->m_selectionGranularity != CHARACTER)
sel.expandUsingGranularity(d->m_selectionGranularity);
- }
- if (shouldChangeSelection(sel)) {
+ if (shouldChangeSelection(sel))
setSelection(sel);
- }
#endif // KHTML_NO_SELECTION
}
void KHTMLPart::khtmlMouseMoveEvent(khtml::MouseMoveEvent *event)
{
-
handleMouseMoveEventSelection(event);
}
1.105 +101 -105 WebCore/khtml/editing/SelectionController.cpp
Index: SelectionController.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/SelectionController.cpp,v
retrieving revision 1.104
retrieving revision 1.105
diff -u -r1.104 -r1.105
--- SelectionController.cpp 4 Dec 2005 00:48:05 -0000 1.104
+++ SelectionController.cpp 6 Dec 2005 04:29:24 -0000 1.105
@@ -102,7 +102,7 @@
: m_base(o.m_base), m_extent(o.m_extent)
, m_start(o.m_start), m_end(o.m_end)
, m_state(o.m_state), m_affinity(o.m_affinity)
- , m_baseIsStart(o.m_baseIsStart)
+ , m_baseIsFirst(o.m_baseIsFirst)
, m_needsLayout(o.m_needsLayout)
, m_modifyBiasSet(o.m_modifyBiasSet)
{
@@ -121,7 +121,7 @@
{
// FIXME: set extentAffinity
m_state = NONE;
- m_baseIsStart = true;
+ m_baseIsFirst = true;
m_affinity = affinity;
m_needsLayout = true;
m_modifyBiasSet = false;
@@ -137,7 +137,7 @@
m_state = o.m_state;
m_affinity = o.m_affinity;
- m_baseIsStart = o.m_baseIsStart;
+ m_baseIsFirst = o.m_baseIsFirst;
m_needsLayout = o.m_needsLayout;
m_modifyBiasSet = o.m_modifyBiasSet;
@@ -491,61 +491,8 @@
{
if (isNone())
return false;
-
- switch (granularity) {
- case CHARACTER:
- break;
- case WORD: {
- // General case: Select the word the caret is positioned inside of, or at the start of (RightWordIfOnBoundary).
- // Edge case: If the caret is after the last word in a soft-wrapped line or the last word in
- // the document, select that last word (LeftWordIfOnBoundary).
- // Edge case: If the caret is after the last word in a paragraph, select from the the end of the
- // last word to the line break (also RightWordIfOnBoundary);
- VisiblePosition start = VisiblePosition(m_start, m_affinity);
- VisiblePosition end = VisiblePosition(m_end, m_affinity);
- EWordSide side = RightWordIfOnBoundary;
- if (isEndOfDocument(start) || (isEndOfLine(start) && !isStartOfLine(start) && !isEndOfParagraph(start)))
- side = LeftWordIfOnBoundary;
- m_start = startOfWord(start, side).deepEquivalent();
- side = RightWordIfOnBoundary;
- if (isEndOfDocument(end) || (isEndOfLine(end) && !isStartOfLine(end) && !isEndOfParagraph(end)))
- side = LeftWordIfOnBoundary;
- m_end = endOfWord(end, side).deepEquivalent();
-
- break;
- }
- case LINE:
- case LINE_BOUNDARY:
- m_start = startOfLine(VisiblePosition(m_start, m_affinity)).deepEquivalent();
- m_end = endOfLine(VisiblePosition(m_end, m_affinity), IncludeLineBreak).deepEquivalent();
- break;
- case PARAGRAPH: {
- VisiblePosition pos(m_start, m_affinity);
- if (isStartOfLine(pos) && isEndOfDocument(pos))
- pos = pos.previous();
- m_start = startOfParagraph(pos).deepEquivalent();
- m_end = endOfParagraph(VisiblePosition(m_end, m_affinity), IncludeLineBreak).deepEquivalent();
- break;
- }
- case DOCUMENT_BOUNDARY:
- m_start = startOfDocument(VisiblePosition(m_start, m_affinity)).deepEquivalent();
- m_end = endOfDocument(VisiblePosition(m_end, m_affinity)).deepEquivalent();
- break;
- case PARAGRAPH_BOUNDARY:
- m_start = startOfParagraph(VisiblePosition(m_start, m_affinity)).deepEquivalent();
- m_end = endOfParagraph(VisiblePosition(m_end, m_affinity)).deepEquivalent();
- break;
- }
- if (m_baseIsStart) {
- m_base = m_start;
- m_extent = m_end;
- } else {
- m_base = m_end;
- m_extent = m_start;
- }
-
- validate();
+ validate(granularity);
return true;
}
@@ -824,59 +771,64 @@
p->fillRect(caret & rect, QBrush());
}
-void SelectionController::adjustExtentForEditableContent()
+void SelectionController::adjustForEditableContent()
{
- Position base = this->base();
- Position extent = this->extent();
-
- if (!base.node() || !extent.node())
+ if (m_base.isNull())
return;
+
+ NodeImpl *baseRoot = m_base.node()->rootEditableElement();
+ NodeImpl *startRoot = m_start.node()->rootEditableElement();
+ NodeImpl *endRoot = m_end.node()->rootEditableElement();
- NodeImpl *baseRoot = base.node()->rootEditableElement();
- NodeImpl *extentRoot = extent.node()->rootEditableElement();
-
- if (baseRoot == extentRoot)
+ // The base, start and end are all in the same region. No adjustment necessary.
+ if (baseRoot == startRoot && baseRoot == endRoot)
return;
- bool baseIsStart = RangeImpl::compareBoundaryPoints(base, extent) <= 0;
-
- // base is in an editable region, but extent is not.
+ // The selection is based in an editable area. Keep both sides from reaching outside that area.
if (baseRoot) {
- Position first(Position(baseRoot, 0));
- Position last(Position(baseRoot, baseRoot->maxDeepOffset()));
-
- m_extent = baseIsStart ? last : first;
- // extent is in an editable region, but base is not.
+ // If the start is outside the base's editable root, cap it at the start of that editable root.
+ if (baseRoot != startRoot) {
+ VisiblePosition first(Position(baseRoot, 0));
+ m_start = first.deepEquivalent();
+ }
+ // If the end is outside the base's editable root, cap it at the end of that editable root.
+ if (baseRoot != endRoot) {
+ VisiblePosition last(Position(baseRoot, baseRoot->maxDeepOffset()));
+ m_end = last.deepEquivalent();
+ }
+ // The selection is based outside editable content. Keep both sides from reaching into editable content.
} else {
- if (baseIsStart) {
+ // The selection ends in editable content, move backward until non-editable content is reached.
+ if (endRoot) {
VisiblePosition previous;
do {
- previous = VisiblePosition(Position(extentRoot, 0)).previous();
- extentRoot = previous.deepEquivalent().node()->rootEditableElement();
- } while (extentRoot);
+ previous = VisiblePosition(Position(endRoot, 0)).previous();
+ endRoot = previous.deepEquivalent().node()->rootEditableElement();
+ } while (endRoot);
- // Since we know that base is before extent, and since we know that base is not in a content editable element,
- // we know that we must have reached non editable content.
ASSERT(!previous.isNull());
- m_extent = previous.deepEquivalent();
- } else {
+ m_end = previous.deepEquivalent();
+ }
+ // The selection starts in editable content, move forward until non-editable content is reached.
+ if (startRoot) {
VisiblePosition next;
do {
- next = VisiblePosition(Position(extentRoot, extentRoot->maxDeepOffset())).next();
- extentRoot = next.deepEquivalent().node()->rootEditableElement();
- } while (extentRoot);
+ next = VisiblePosition(Position(startRoot, startRoot->maxDeepOffset())).next();
+ startRoot = next.deepEquivalent().node()->rootEditableElement();
+ } while (startRoot);
- // Since we know that extent is before base, and since we know that extent is not in a content editable element,
- // we know that we must have reached non editable content.
ASSERT(!next.isNull());
- m_extent = next.deepEquivalent();
+ m_start = next.deepEquivalent();
}
}
+
+ // Correct the extent if necessary.
+ if (baseRoot != m_extent.node()->rootEditableElement())
+ m_extent = m_baseIsFirst ? m_end : m_start;
}
-void SelectionController::validate()
+void SelectionController::validate(ETextGranularity granularity)
{
- adjustExtentForEditableContent();
// Move the selection to rendered positions, if possible.
Position originalBase(m_base);
bool baseAndExtentEqual = m_base == m_extent;
@@ -911,37 +863,81 @@
m_start.clear();
m_end.clear();
}
- m_baseIsStart = true;
- }
- else if (m_base.isNull()) {
+ m_baseIsFirst = true;
+ } else if (m_base.isNull()) {
m_base = m_extent;
- m_baseIsStart = true;
- }
- else if (m_extent.isNull()) {
+ m_baseIsFirst = true;
+ } else if (m_extent.isNull()) {
m_extent = m_base;
- m_baseIsStart = true;
- }
- else {
- m_baseIsStart = RangeImpl::compareBoundaryPoints(m_base.node(), m_base.offset(), m_extent.node(), m_extent.offset()) <= 0;
+ m_baseIsFirst = true;
+ } else {
+ m_baseIsFirst = RangeImpl::compareBoundaryPoints(m_base.node(), m_base.offset(), m_extent.node(), m_extent.offset()) <= 0;
}
- if (m_baseIsStart) {
+ if (m_baseIsFirst) {
m_start = m_base;
m_end = m_extent;
} else {
m_start = m_extent;
m_end = m_base;
}
+
+ // Expand the selection if requested.
+ switch (granularity) {
+ case CHARACTER:
+ // Don't do any expansion.
+ break;
+ case WORD: {
+ // General case: Select the word the caret is positioned inside of, or at the start of (RightWordIfOnBoundary).
+ // Edge case: If the caret is after the last word in a soft-wrapped line or the last word in
+ // the document, select that last word (LeftWordIfOnBoundary).
+ // Edge case: If the caret is after the last word in a paragraph, select from the the end of the
+ // last word to the line break (also RightWordIfOnBoundary);
+ VisiblePosition start = VisiblePosition(m_start, m_affinity);
+ VisiblePosition end = VisiblePosition(m_end, m_affinity);
+ EWordSide side = RightWordIfOnBoundary;
+ if (isEndOfDocument(start) || (isEndOfLine(start) && !isStartOfLine(start) && !isEndOfParagraph(start)))
+ side = LeftWordIfOnBoundary;
+ m_start = startOfWord(start, side).deepEquivalent();
+ side = RightWordIfOnBoundary;
+ if (isEndOfDocument(end) || (isEndOfLine(end) && !isStartOfLine(end) && !isEndOfParagraph(end)))
+ side = LeftWordIfOnBoundary;
+ m_end = endOfWord(end, side).deepEquivalent();
+
+ break;
+ }
+ case LINE:
+ case LINE_BOUNDARY:
+ m_start = startOfLine(VisiblePosition(m_start, m_affinity)).deepEquivalent();
+ m_end = endOfLine(VisiblePosition(m_end, m_affinity), IncludeLineBreak).deepEquivalent();
+ break;
+ case PARAGRAPH: {
+ VisiblePosition pos(m_start, m_affinity);
+ if (isStartOfLine(pos) && isEndOfDocument(pos))
+ pos = pos.previous();
+ m_start = startOfParagraph(pos).deepEquivalent();
+ m_end = endOfParagraph(VisiblePosition(m_end, m_affinity), IncludeLineBreak).deepEquivalent();
+ break;
+ }
+ case DOCUMENT_BOUNDARY:
+ m_start = startOfDocument(VisiblePosition(m_start, m_affinity)).deepEquivalent();
+ m_end = endOfDocument(VisiblePosition(m_end, m_affinity)).deepEquivalent();
+ break;
+ case PARAGRAPH_BOUNDARY:
+ m_start = startOfParagraph(VisiblePosition(m_start, m_affinity)).deepEquivalent();
+ m_end = endOfParagraph(VisiblePosition(m_end, m_affinity)).deepEquivalent();
+ break;
+ }
+
+ adjustForEditableContent();
// adjust the state
if (m_start.isNull()) {
ASSERT(m_end.isNull());
m_state = NONE;
- }
- else if (m_start == m_end || m_start.upstream() == m_end.upstream()) {
+ } else if (m_start == m_end || m_start.upstream() == m_end.upstream()) {
m_state = CARET;
- }
- else {
+ } else {
m_state = RANGE;
// "Constrain" the selection to be the smallest equivalent range of nodes.
// This is a somewhat arbitrary choice, but experience shows that it is
1.47 +3 -3 WebCore/khtml/editing/SelectionController.h
Index: SelectionController.h
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/SelectionController.h,v
retrieving revision 1.46
retrieving revision 1.47
diff -u -r1.46 -r1.47
--- SelectionController.h 4 Dec 2005 00:48:05 -0000 1.46
+++ SelectionController.h 6 Dec 2005 04:29:24 -0000 1.47
@@ -121,8 +121,8 @@
enum EPositionType { START, END, BASE, EXTENT };
void init(EAffinity affinity);
- void validate();
- void adjustExtentForEditableContent();
+ void validate(ETextGranularity granularity = CHARACTER);
+ void adjustForEditableContent();
VisiblePosition modifyExtendingRightForward(ETextGranularity);
VisiblePosition modifyMovingRightForward(ETextGranularity);
@@ -151,7 +151,7 @@
// This is faster than doing another layout().
QPoint m_caretPositionOnLayout;
- bool m_baseIsStart : 1; // true if base node is before the extent node
+ bool m_baseIsFirst : 1; // true if base is before the extent
bool m_needsLayout : 1; // true if the caret and expectedVisible rectangles need to be calculated
bool m_modifyBiasSet : 1; // true if the selection has been horizontally
// modified with EAlter::EXTEND
More information about the webkit-changes
mailing list