newline handling changes to pass more W3C DOM Level 1 Core tests
Darin Adler
darin at apple.com
Tue Oct 7 09:48:54 CEST 2003
In my free time, I've been working on fixes for some of the failures we
see in the W3C DOM Level 1 Core tests. Some of those fixes were done
before we got into our new habit of posting our patches to this list;
I'll either make patches for those later or you'll see them in one of
our tarballs.
One of the areas causing failures was the change of newline characters
into spaces. Here's the KHTML part of changing that.
This change causes the newlines to be left as-is in the DOM. For KDE
you'd need one other change that I have not yet done, to treat newlines
as spaces when measuring and drawing text (presumably changes in
font.cpp). For Mac OS X, I did that at a lower level in the text
drawing machinery since there was already code at that level to handle
non-breaking spaces in a similar way.
The refactoring of KHTMLPart::selectedText() into KHTMLPart::text(const
DOM::Range&) is something I did so I could change some code in KWQ to
share the guts of selectedText() and get rid of some similar but less
capable code in there.
-------------- next part --------------
Index: khtml/rendering/bidi.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/bidi.cpp,v
retrieving revision 1.75
diff -p -u -u -r1.75 bidi.cpp
--- bidi.cpp 2003/10/07 04:43:23 1.75
+++ bidi.cpp 2003/10/07 15:37:13
@@ -369,9 +369,11 @@ static void addRun(BidiRun* bidiRun)
// Compute the number of spaces in this run,
if (bidiRun->obj && bidiRun->obj->isText()) {
RenderText* text = static_cast<RenderText*>(bidiRun->obj);
- for (int i = bidiRun->start; i < bidiRun->stop; i++)
- if (text->text()[i].unicode() == ' ')
+ for (int i = bidiRun->start; i < bidiRun->stop; i++) {
+ const QChar c = text->text()[i];
+ if (c == ' ' || c == '\n')
numSpaces++;
+ }
}
}
@@ -736,9 +738,11 @@ void RenderBlock::computeHorizontalPosit
if (numSpaces > 0 && r->obj->isText() && !r->compact) {
// get the number of spaces in the run
int spaces = 0;
- for ( int i = r->start; i < r->stop; i++ )
- if ( static_cast<RenderText *>(r->obj)->text()[i].unicode() == ' ' )
+ for ( int i = r->start; i < r->stop; i++ ) {
+ const QChar c = static_cast<RenderText *>(r->obj)->text()[i];
+ if (c == ' ' || c == '\n')
spaces++;
+ }
KHTMLAssert(spaces <= numSpaces);
@@ -1443,8 +1447,8 @@ BidiIterator RenderBlock::findNextLineBr
// eliminate spaces at beginning of line
// remove leading spaces. Any inline flows we encounter will be empty and should also
// be skipped.
- while (!start.atEnd() && (start.obj->isInlineFlow() || (start.obj->style()->whiteSpace() != PRE &&
- (start.current() == ' ' || start.obj->isFloatingOrPositioned())))) {
+ while (!start.atEnd() && (start.obj->isInlineFlow() || (start.obj->style()->whiteSpace() != PRE && !start.obj->isBR() &&
+ (start.current() == ' ' || start.current() == '\n' || start.obj->isFloatingOrPositioned())))) {
if( start.obj->isFloatingOrPositioned() ) {
RenderObject *o = start.obj;
// add to special objects...
@@ -1625,20 +1629,15 @@ BidiIterator RenderBlock::findNextLineBr
bool appliedEndWidth = false;
while(len) {
- //XXXdwh This is wrong. Still mutating the DOM
- // string for newlines... will fix in second stage.
- if (!isPre && str[pos] == '\n'){
- str[pos] = ' ';
- }
-
bool previousCharacterIsSpace = currentCharacterIsSpace;
- currentCharacterIsSpace = (str[pos].unicode() == ' ');
-
+ const QChar c = str[pos];
+ currentCharacterIsSpace = c == ' ' || (!isPre && c == '\n');
+
if (isPre || !currentCharacterIsSpace)
isLineEmpty = false;
bool applyWordSpacing = false;
- if( (isPre && str[pos] == '\n') || (!isPre && isBreakable( str, pos, strlen )) ) {
+ if ( (isPre && c == '\n') || (!isPre && isBreakable( str, pos, strlen )) ) {
if (ignoringSpaces) {
if (!currentCharacterIsSpace) {
// Stop ignoring spaces and begin at this
Index: khtml/rendering/render_text.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_text.cpp,v
retrieving revision 1.83
diff -p -u -u -r1.83 render_text.cpp
--- render_text.cpp 2003/10/01 20:58:55 1.83
+++ render_text.cpp 2003/10/07 15:37:14
@@ -945,7 +945,7 @@ void RenderText::trimmedMinMaxWidth(shor
hasBreakableChar = m_hasBreakableChar;
hasBreak = m_hasBreak;
- if (stripFrontSpaces && str->s[0].unicode() == ' ') {
+ if (stripFrontSpaces && (str->s[0] == ' ' || (!isPre && str->s[0] == '\n'))) {
const Font *f = htmlFont( false );
QChar space[1]; space[0] = ' ';
int spaceWidth = f->width(space, 1, 0);
@@ -1021,21 +1021,23 @@ void RenderText::calcMinMaxWidth()
bool firstWord = true;
for(int i = 0; i < len; i++)
{
+ const QChar c = str->s[i];
+
+ bool previousCharacterIsSpace = isSpace;
+
bool isNewline = false;
- // XXXdwh Wrong in the first stage. Will stop mutating newlines
- // in a second stage.
- if (str->s[i] == '\n') {
+ if (c == '\n') {
if (isPre) {
m_hasBreak = true;
isNewline = true;
+ isSpace = false;
}
else
- str->s[i] = ' ';
+ isSpace = true;
+ } else {
+ isSpace = c == ' ';
}
- bool previousCharacterIsSpace = isSpace;
- isSpace = str->s[i].unicode() == ' ';
-
if ((isSpace || isNewline) && i == 0)
m_hasBeginWS = true;
if ((isSpace || isNewline) && i == len-1)
@@ -1230,7 +1232,7 @@ void RenderText::position(InlineBox* box
InlineTextBox *s = static_cast<InlineTextBox*>(box);
// ### should not be needed!!!
- if (len == 0 || (str->l && len == 1 && *(str->s+from) == '\n')) {
+ if (len == 0 || isBR()) {
// We want the box to be destroyed. This is a <br>, and we don't
// need <br>s to be included.
s->detach(renderArena());
Index: khtml/khtml_part.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/khtml_part.h,v
retrieving revision 1.49
retrieving revision 1.50
diff -p -u -r1.49 -r1.50
--- khtml_part.h 2003/09/24 15:55:19 1.49
+++ khtml_part.h 2003/10/07 00:01:43 1.50
@@ -563,6 +563,11 @@ public:
void setSelection( const DOM::Range & );
/**
+ * Returns the text for a part of the document.
+ */
+ QString text(const DOM::Range &) const;
+
+ /**
* Has the user selected anything?
*
* Call @ref selectedText() to
Index: khtml/khtml_part.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/khtml_part.cpp,v
retrieving revision 1.153
retrieving revision 1.154
diff -p -u -r1.153 -r1.154
--- khtml_part.cpp 2003/09/25 16:51:10 1.153
+++ khtml_part.cpp 2003/10/07 00:01:43 1.154
@@ -2216,7 +2216,7 @@ bool KHTMLPart::findTextNext( const QStr
}
}
-QString KHTMLPart::selectedText() const
+QString KHTMLPart::text(const DOM::Range &r) const
{
// FIXME: This whole function should use the render tree and not the DOM tree, since elements could
// be hidden using CSS, or additional generated content could be added. For now, we just make sure
@@ -2225,7 +2225,7 @@ QString KHTMLPart::selectedText() const
bool hasNewLine = true;
bool addedSpace = true;
QString text;
- DOM::Node n = d->m_selectionStart;
+ DOM::Node n = r.startContainer();
while(!n.isNull()) {
if(n.nodeType() == DOM::Node::TEXT_NODE) {
if (hasNewLine) {
@@ -2233,8 +2233,8 @@ QString KHTMLPart::selectedText() const
hasNewLine = false;
}
QString str = n.nodeValue().string();
- int start = (n == d->m_selectionStart) ? d->m_startOffset : -1;
- int end = (n == d->m_selectionEnd) ? d->m_endOffset : -1;
+ int start = (n == r.startContainer()) ? r.startOffset() : -1;
+ int end = (n == r.endContainer()) ? r.endOffset() : -1;
RenderObject* renderer = n.handle()->renderer();
if (renderer && renderer->isText()) {
if (renderer->style()->whiteSpace() == khtml::PRE) {
@@ -2261,7 +2261,9 @@ QString KHTMLPart::selectedText() const
bool spaceBetweenRuns = false;
if (runStart >= runs[i]->m_start &&
runStart < runs[i]->m_start + runs[i]->m_len) {
- text += str.mid(runStart, runEnd - runStart);
+ QString runText = str.mid(runStart, runEnd - runStart);
+ runText.replace('\n', ' ');
+ text += runText;
start = -1;
spaceBetweenRuns = i+1 < runs.count() && runs[i+1]->m_start > runEnd;
addedSpace = str[runEnd-1].direction() == QChar::DirWS;
@@ -2318,7 +2320,7 @@ QString KHTMLPart::selectedText() const
break;
}
}
- if(n == d->m_selectionEnd) break;
+ if(n == r.endContainer()) break;
DOM::Node next = n.firstChild();
if(next.isNull()) next = n.nextSibling();
while( next.isNull() && !n.parentNode().isNull() ) {
@@ -2375,6 +2377,11 @@ QString KHTMLPart::selectedText() const
return text.mid(start, end-start);
}
+QString KHTMLPart::selectedText() const
+{
+ return text(selection());
+}
+
bool KHTMLPart::hasSelection() const
{
return ( !d->m_selectionStart.isNull() &&
@@ -2383,7 +2390,7 @@ bool KHTMLPart::hasSelection() const
DOM::Range KHTMLPart::selection() const
{
- DOM::Range r = document().createRange();DOM::Range();
+ DOM::Range r = document().createRange();
r.setStart( d->m_selectionStart, d->m_startOffset );
r.setEnd( d->m_selectionEnd, d->m_endOffset );
return r;
-------------- next part --------------
-- Darin
More information about the Khtml-devel
mailing list