[webkit-changes] cvs commit: WebCore/kwq KWQKHTMLPart.mm
WebCoreBridge.mm
David
harrison at opensource.apple.com
Thu Sep 1 11:03:36 PDT 2005
harrison 05/09/01 11:03:36
Modified: . ChangeLog
khtml khtml_part.cpp
khtml/editing selection.h visible_position.cpp
visible_position.h visible_units.cpp
khtml/rendering render_text.cpp
kwq KWQKHTMLPart.mm WebCoreBridge.mm
Log:
Reviewed by Justin.
<rdar://problem/4054701> assertion failure in khtml::isEqualIgnoringAffinity using VoiceOver in new Mail message
Problem was that an AXTextMarker was erroneously given UPSTREAM affinity. Fixed by having the
VisiblePosition constructors make the actual affinity DOWNSTREAM if UPSTREAM was specified, but
the Position is not at a line wrap.
Test cases added:
There is no way to automate a test for this because it requires using the AX APIs, which are
not available to the tests.
A manual test involves creating an email and using VoiceOver on it. Seems like too much.
* khtml/editing/selection.h:
* khtml/editing/visible_position.cpp:
(khtml::VisiblePosition::init):
(khtml::VisiblePosition::next):
* khtml/editing/visible_position.h:
* khtml/editing/visible_units.cpp:
(khtml::nextBoundary):
(khtml::endOfLine):
* khtml/khtml_part.cpp:
(KHTMLPart::findTextNext):
(KHTMLPart::selectFrameElementInParentIfFullySelected):
* khtml/rendering/render_text.cpp:
(RenderText::positionForCoordinates):
* kwq/KWQKHTMLPart.mm:
(KWQKHTMLPart::findString):
(KWQKHTMLPart::advanceToNextMisspelling):
* kwq/WebCoreBridge.mm:
(-[WebCoreBridge setSelectedDOMRange:affinity:closeTyping:]):
Revision Changes Path
1.71 +34 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.70
retrieving revision 1.71
diff -u -r1.70 -r1.71
--- ChangeLog 1 Sep 2005 17:40:12 -0000 1.70
+++ ChangeLog 1 Sep 2005 18:03:30 -0000 1.71
@@ -1,3 +1,37 @@
+2005-09-01 David Harrison <harrison at apple.com>
+
+ Reviewed by Justin.
+
+ <rdar://problem/4054701> assertion failure in khtml::isEqualIgnoringAffinity using VoiceOver in new Mail message
+
+ Problem was that an AXTextMarker was erroneously given UPSTREAM affinity. Fixed by having the
+ VisiblePosition constructors make the actual affinity DOWNSTREAM if UPSTREAM was specified, but
+ the Position is not at a line wrap.
+
+ Test cases added:
+ There is no way to automate a test for this because it requires using the AX APIs, which are
+ not available to the tests.
+ A manual test involves creating an email and using VoiceOver on it. Seems like too much.
+
+ * khtml/editing/selection.h:
+ * khtml/editing/visible_position.cpp:
+ (khtml::VisiblePosition::init):
+ (khtml::VisiblePosition::next):
+ * khtml/editing/visible_position.h:
+ * khtml/editing/visible_units.cpp:
+ (khtml::nextBoundary):
+ (khtml::endOfLine):
+ * khtml/khtml_part.cpp:
+ (KHTMLPart::findTextNext):
+ (KHTMLPart::selectFrameElementInParentIfFullySelected):
+ * khtml/rendering/render_text.cpp:
+ (RenderText::positionForCoordinates):
+ * kwq/KWQKHTMLPart.mm:
+ (KWQKHTMLPart::findString):
+ (KWQKHTMLPart::advanceToNextMisspelling):
+ * kwq/WebCoreBridge.mm:
+ (-[WebCoreBridge setSelectedDOMRange:affinity:closeTyping:]):
+
2005-08-31 Adele Peterson <adele at apple.com>
Reviewed by Dave Hyatt.
1.339 +2 -2 WebCore/khtml/khtml_part.cpp
Index: khtml_part.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/khtml_part.cpp,v
retrieving revision 1.338
retrieving revision 1.339
diff -u -r1.338 -r1.339
--- khtml_part.cpp 31 Aug 2005 04:38:35 -0000 1.338
+++ khtml_part.cpp 1 Sep 2005 18:03:31 -0000 1.339
@@ -2398,7 +2398,7 @@
d->m_view->setContentsPos(x-50, y-50);
Position p1(d->m_findNode, d->m_findPos);
Position p2(d->m_findNode, d->m_findPos + matchLen);
- Selection sel = Selection(p1, khtml::DOWNSTREAM, p2, khtml::SEL_PREFER_UPSTREAM_AFFINITY);
+ Selection sel = Selection(p1, khtml::DOWNSTREAM, p2, khtml::VP_UPSTREAM_IF_POSSIBLE);
if (shouldChangeSelection(sel)) {
setSelection(sel);
}
@@ -5985,7 +5985,7 @@
// Create compute positions before and after the element.
unsigned long ownerElementNodeIndex = ownerElement->nodeIndex();
VisiblePosition beforeOwnerElement(VisiblePosition(ownerElementParent, ownerElementNodeIndex, khtml::SEL_DEFAULT_AFFINITY));
- VisiblePosition afterOwnerElement(VisiblePosition(ownerElementParent, ownerElementNodeIndex + 1, khtml::SEL_PREFER_UPSTREAM_AFFINITY));
+ VisiblePosition afterOwnerElement(VisiblePosition(ownerElementParent, ownerElementNodeIndex + 1, khtml::VP_UPSTREAM_IF_POSSIBLE));
// Focus on the parent frame, and then select from before this element to after.
if (parent->shouldChangeSelection(Selection(beforeOwnerElement, afterOwnerElement))) {
1.40 +0 -4 WebCore/khtml/editing/selection.h
Index: selection.h
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/selection.h,v
retrieving revision 1.39
retrieving revision 1.40
diff -u -r1.39 -r1.40
--- selection.h 23 Jun 2005 19:47:48 -0000 1.39
+++ selection.h 1 Sep 2005 18:03:32 -0000 1.40
@@ -47,10 +47,6 @@
enum EDirection { FORWARD, BACKWARD, RIGHT, LEFT };
#define SEL_DEFAULT_AFFINITY DOWNSTREAM
-// FIXME: Implement as "caller does not know whether it is OK to be upstream,
-// but that would be the desired affinity"
-#define SEL_PREFER_UPSTREAM_AFFINITY DOWNSTREAM
-
typedef DOM::Position Position;
typedef DOM::RangeImpl RangeImpl;
1.57 +5 -14 WebCore/khtml/editing/visible_position.cpp
Index: visible_position.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/visible_position.cpp,v
retrieving revision 1.56
retrieving revision 1.57
diff -u -r1.56 -r1.57
--- visible_position.cpp 25 Aug 2005 23:13:45 -0000 1.56
+++ visible_position.cpp 1 Sep 2005 18:03:32 -0000 1.57
@@ -80,6 +80,10 @@
initUpstream(pos);
else
initDownstream(pos);
+
+ // when not at line break, make sure to end up with DOWNSTREAM affinity.
+ if (m_affinity == UPSTREAM && (isNull() || inSameLine(VisiblePosition(pos, DOWNSTREAM), *this)))
+ m_affinity = DOWNSTREAM;
}
void VisiblePosition::initUpstream(const Position &pos)
@@ -153,9 +157,7 @@
VisiblePosition VisiblePosition::next() const
{
- VisiblePosition result = VisiblePosition(nextVisiblePosition(m_deepPosition), m_affinity);
- setAffinityUsingLinePosition(result);
- return result;
+ return VisiblePosition(nextVisiblePosition(m_deepPosition), m_affinity);
}
VisiblePosition VisiblePosition::previous() const
@@ -457,17 +459,6 @@
return code == 0;
}
-void setAffinityUsingLinePosition(VisiblePosition &pos)
-{
- // When not moving across line wrap, make sure to end up with DOWNSTREAM affinity.
- if (pos.isNotNull() && pos.affinity() == UPSTREAM) {
- VisiblePosition temp(pos);
- temp.setAffinity(DOWNSTREAM);
- if (inSameLine(temp, pos))
- pos.setAffinity(DOWNSTREAM);
- }
-}
-
DOM::NodeImpl *enclosingBlockFlowElement(const VisiblePosition &vp)
{
if (vp.isNull())
1.30 +13 -2 WebCore/khtml/editing/visible_position.h
Index: visible_position.h
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/visible_position.h,v
retrieving revision 1.29
retrieving revision 1.30
diff -u -r1.29 -r1.30
--- visible_position.h 23 Jun 2005 19:47:49 -0000 1.29
+++ visible_position.h 1 Sep 2005 18:03:33 -0000 1.30
@@ -39,14 +39,27 @@
namespace khtml {
+// VisiblePosition default affinity is downstream because
+// the callers do not really care (they just want the
+// deep position without regard to line position), and this
+// is cheaper than UPSTREAM
#define VP_DEFAULT_AFFINITY DOWNSTREAM
+// Callers who do not know where on the line the position is,
+// but would like UPSTREAM if at a line break or DOWNSTREAM
+// otherwise, need a clear way to specify that. The
+// constructors auto-correct UPSTREAM to DOWNSTREAM if the
+// position is not at a line break.
+#define VP_UPSTREAM_IF_POSSIBLE UPSTREAM
+
class VisiblePosition
{
public:
typedef DOM::NodeImpl NodeImpl;
typedef DOM::Position Position;
+ // NOTE: UPSTREAM affinity will be used only if pos is at end of a wrapped line,
+ // otherwise it will be converted to DOWNSTREAM
VisiblePosition() { m_affinity = VP_DEFAULT_AFFINITY; };
VisiblePosition(NodeImpl *, long offset, EAffinity);
VisiblePosition(const Position &, EAffinity);
@@ -121,8 +134,6 @@
VisiblePosition startVisiblePosition(const DOM::RangeImpl *, EAffinity);
VisiblePosition endVisiblePosition(const DOM::RangeImpl *, EAffinity);
-void setAffinityUsingLinePosition(VisiblePosition &);
-
DOM::NodeImpl *enclosingBlockFlowElement(const VisiblePosition &);
bool isFirstVisiblePositionInNode(const VisiblePosition &, const DOM::NodeImpl *);
1.39 +5 -9 WebCore/khtml/editing/visible_units.cpp
Index: visible_units.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/visible_units.cpp,v
retrieving revision 1.38
retrieving revision 1.39
diff -u -r1.38 -r1.39
--- visible_units.cpp 25 Aug 2005 23:13:46 -0000 1.38
+++ visible_units.cpp 1 Sep 2005 18:03:33 -0000 1.39
@@ -190,7 +190,9 @@
charIt.advance(next - 1);
pos = Position(charIt.range()->endContainer(exception), charIt.range()->endOffset(exception));
}
- return VisiblePosition(pos, UPSTREAM);
+
+ // generate VisiblePosition, use UPSTREAM affinity if possible
+ return VisiblePosition(pos, VP_UPSTREAM_IF_POSSIBLE);
}
// ---------
@@ -341,14 +343,8 @@
endOffset = endTextBox->m_start + endTextBox->m_len;
}
- // generate VisiblePosition with correct affinity
- VisiblePosition result = VisiblePosition(endNode, endOffset, DOWNSTREAM);
- VisiblePosition temp = result;
- temp.setAffinity(UPSTREAM);
- if (!inSameLine(temp, result))
- result.setAffinity(UPSTREAM);
-
- return result;
+ // generate VisiblePosition, use UPSTREAM affinity if possible
+ return VisiblePosition(endNode, endOffset, VP_UPSTREAM_IF_POSSIBLE);
}
bool inSameLine(const VisiblePosition &a, const VisiblePosition &b)
1.195 +4 -9 WebCore/khtml/rendering/render_text.cpp
Index: render_text.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/rendering/render_text.cpp,v
retrieving revision 1.194
retrieving revision 1.195
diff -u -r1.194 -r1.195
--- render_text.cpp 1 Sep 2005 16:21:05 -0000 1.194
+++ render_text.cpp 1 Sep 2005 18:03:33 -0000 1.195
@@ -960,12 +960,8 @@
// and the x coordinate is to the left of the right edge of this box
// check to see if position goes in this box
int offset = box->offsetForPosition(_x - absx);
- if (offset != -1) {
- EAffinity affinity = offset >= box->m_len && !box->nextOnLine() ? UPSTREAM : DOWNSTREAM;
- VisiblePosition result = VisiblePosition(element(), offset + box->m_start, affinity);
- setAffinityUsingLinePosition(result);
- return result;
- }
+ if (offset != -1)
+ return VisiblePosition(element(), offset + box->m_start, VP_UPSTREAM_IF_POSSIBLE);
}
else if (!box->prevOnLine() && _x < absx + box->m_x) {
// box is first on line
@@ -975,9 +971,8 @@
else if (!box->nextOnLine() && _x >= absx + box->m_x + box->m_width) {
// box is last on line
// and the x coordinate is to the right of the last text box right edge
- VisiblePosition result = VisiblePosition(element(), box->m_start + box->m_len, UPSTREAM);
- setAffinityUsingLinePosition(result);
- return result;
+ // generate VisiblePosition, use UPSTREAM affinity if possible
+ return VisiblePosition(element(), box->m_start + box->m_len, VP_UPSTREAM_IF_POSSIBLE);
}
}
}
1.669 +3 -2 WebCore/kwq/KWQKHTMLPart.mm
Index: KWQKHTMLPart.mm
===================================================================
RCS file: /cvs/root/WebCore/kwq/KWQKHTMLPart.mm,v
retrieving revision 1.668
retrieving revision 1.669
diff -u -r1.668 -r1.669
--- KWQKHTMLPart.mm 31 Aug 2005 23:05:00 -0000 1.668
+++ KWQKHTMLPart.mm 1 Sep 2005 18:03:34 -0000 1.669
@@ -150,6 +150,7 @@
using khtml::StyleDashboardRegion;
using khtml::TextIterator;
using khtml::DOWNSTREAM;
+using khtml::VP_UPSTREAM_IF_POSSIBLE;
using khtml::VISIBLE;
using khtml::VisiblePosition;
using khtml::WordAwareIterator;
@@ -654,7 +655,7 @@
return false;
}
- setSelection(Selection(resultRange.get(), DOWNSTREAM, khtml::SEL_PREFER_UPSTREAM_AFFINITY));
+ setSelection(Selection(resultRange.get(), DOWNSTREAM, VP_UPSTREAM_IF_POSSIBLE));
jumpToSelection();
return true;
}
@@ -1021,7 +1022,7 @@
QString result = chars.string(misspelling.length);
misspellingRange->setEnd(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
- setSelection(Selection(misspellingRange.get(), DOWNSTREAM, khtml::SEL_PREFER_UPSTREAM_AFFINITY));
+ setSelection(Selection(misspellingRange.get(), DOWNSTREAM, VP_UPSTREAM_IF_POSSIBLE));
jumpToSelection();
// Mark misspelling in document.
xmlDocImpl()->addMarker(misspellingRange.get(), DocumentMarker::Spelling);
1.411 +7 -17 WebCore/kwq/WebCoreBridge.mm
Index: WebCoreBridge.mm
===================================================================
RCS file: /cvs/root/WebCore/kwq/WebCoreBridge.mm,v
retrieving revision 1.410
retrieving revision 1.411
diff -u -r1.410 -r1.411
--- WebCoreBridge.mm 25 Aug 2005 23:13:58 -0000 1.410
+++ WebCoreBridge.mm 1 Sep 2005 18:03:35 -0000 1.411
@@ -124,7 +124,6 @@
using khtml::ReplaceSelectionCommand;
using khtml::Selection;
using khtml::SharedPtr;
-using khtml::setAffinityUsingLinePosition;
using khtml::Tokenizer;
using khtml::TextIterator;
using khtml::TypingCommand;
@@ -1589,25 +1588,16 @@
_part->xmlDocImpl()->updateLayout();
EAffinity affinity = static_cast<EAffinity>(selectionAffinity);
-
- bool rangeCollapsed = [range collapsed];
- if (!rangeCollapsed)
+
+ // Non-collapsed ranges are not allowed to start at the end of a line that is wrapped,
+ // they start at the beginning of the next line instead
+ if (![range collapsed])
affinity = DOWNSTREAM;
- // Work around bug where isRenderedContent returns false for <br> elements at the ends of lines.
- // If that bug wasn't an issue, we could just make the position from the range directly.
- Position start(startContainer, [range startOffset]);
- Position end(endContainer, [range endOffset]);
- VisiblePosition visibleStart(start, affinity);
- start = visibleStart.deepEquivalent();
-
- if (rangeCollapsed) {
- setAffinityUsingLinePosition(visibleStart);
- affinity = visibleStart.affinity();
- }
-
// FIXME: Can we provide extentAffinity?
- Selection selection(start, affinity, end, khtml::SEL_DEFAULT_AFFINITY);
+ VisiblePosition visibleStart(startContainer, [range startOffset], affinity);
+ VisiblePosition visibleEnd(endContainer, [range endOffset], khtml::SEL_DEFAULT_AFFINITY);
+ Selection selection(visibleStart, visibleEnd);
_part->setSelection(selection, closeTyping);
}
More information about the webkit-changes
mailing list