[webkit-changes] cvs commit: WebCore/khtml/rendering
render_block.cpp render_text.cpp render_text.h
David
harrison at opensource.apple.com
Thu Sep 15 16:50:20 PDT 2005
harrison 05/09/15 16:50:20
Modified: . ChangeLog
khtml/rendering render_block.cpp render_text.cpp
render_text.h
Added: layout-tests/editing/selection
extend-by-word-002-expected.checksum
extend-by-word-002-expected.png
extend-by-word-002-expected.txt
extend-by-word-002.html
Log:
Reviewed by Dave Hyatt.
<rdar://problem/4244976> reproducible hang at ocharleys.com in VisiblePosition::initDownstream
Problem is that RenderText::nextOffset() passes an empty string the UBreakIterator, which returns
UBRK_DONE (-1) from ubrk_following, and that -1 is used without question as return result of
nextOffset(). Fixed by checking for UBRK_DONE and returning offset+1 in that case. Similar
change in RenderText::previousOffset().
Test cases added:
* layout-tests/editing/selection/extend-by-word-002-expected.checksum: Added.
* layout-tests/editing/selection/extend-by-word-002-expected.png: Added.
* layout-tests/editing/selection/extend-by-word-002-expected.txt: Added.
* layout-tests/editing/selection/extend-by-word-002.html: Added.
* khtml/rendering/render_block.cpp:
(khtml::RenderBlock::updateFirstLetter):
Added comments. Slight format adjustments.
* khtml/rendering/render_text.cpp:
(getCharacterBreakIterator):
Slight format adjustment.
(RenderText::previousOffset):
(RenderText::nextOffset):
Check for UBRK_DONE.
(RenderTextFragment::RenderTextFragment)
(RenderTextFragment::RenderTextFragment)
Fixed parameter names.
(m_generatedContentStr):
* khtml/rendering/render_text.h:
Fixed parameter names in the two RenderTextFragment constructors.
Revision Changes Path
1.126 +37 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.125
retrieving revision 1.126
diff -u -r1.125 -r1.126
--- ChangeLog 15 Sep 2005 05:22:56 -0000 1.125
+++ ChangeLog 15 Sep 2005 23:50:14 -0000 1.126
@@ -1,3 +1,40 @@
+2005-09-15 David Harrison <harrison at apple.com>
+
+ Reviewed by Dave Hyatt.
+
+ <rdar://problem/4244976> reproducible hang at ocharleys.com in VisiblePosition::initDownstream
+
+ Problem is that RenderText::nextOffset() passes an empty string the UBreakIterator, which returns
+ UBRK_DONE (-1) from ubrk_following, and that -1 is used without question as return result of
+ nextOffset(). Fixed by checking for UBRK_DONE and returning offset+1 in that case. Similar
+ change in RenderText::previousOffset().
+
+ Test cases added:
+ * layout-tests/editing/selection/extend-by-word-002-expected.checksum: Added.
+ * layout-tests/editing/selection/extend-by-word-002-expected.png: Added.
+ * layout-tests/editing/selection/extend-by-word-002-expected.txt: Added.
+ * layout-tests/editing/selection/extend-by-word-002.html: Added.
+
+ * khtml/rendering/render_block.cpp:
+ (khtml::RenderBlock::updateFirstLetter):
+ Added comments. Slight format adjustments.
+
+ * khtml/rendering/render_text.cpp:
+ (getCharacterBreakIterator):
+ Slight format adjustment.
+
+ (RenderText::previousOffset):
+ (RenderText::nextOffset):
+ Check for UBRK_DONE.
+
+ (RenderTextFragment::RenderTextFragment)
+ (RenderTextFragment::RenderTextFragment)
+ Fixed parameter names.
+
+ (m_generatedContentStr):
+ * khtml/rendering/render_text.h:
+ Fixed parameter names in the two RenderTextFragment constructors.
+
2005-09-14 Alexey Proskuryakov <ap at nypop.com>
Reviewed, tweaked, and landed by Darin.
1.1 WebCore/layout-tests/editing/selection/extend-by-word-002-expected.checksum
Index: extend-by-word-002-expected.checksum
===================================================================
057d342d481f917255d4e261d29a5f28
1.1 WebCore/layout-tests/editing/selection/extend-by-word-002-expected.png
<<Binary file>>
1.1 WebCore/layout-tests/editing/selection/extend-by-word-002-expected.txt
Index: extend-by-word-002-expected.txt
===================================================================
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
layer at (0,0) size 800x600
RenderCanvas at (0,0) size 800x600
layer at (0,0) size 800x600
RenderBlock {HTML} at (0,0) size 800x600
RenderBody {BODY} at (8,8) size 784x584
RenderBlock {DIV} at (0,0) size 148x166 [border: (2px solid #FF0000)]
RenderBlock {UL} at (14,14) size 120x138
RenderListItem {LI} at (0,0) size 120x21
RenderListMarker at (5,6) size 0x10
RenderInline (generated) at (0,0) size 14x24
RenderText {TEXT} at (5,-4) size 14x10
text run at (5,-4) width 14: "\x{B7} "
RenderText {TEXT} at (0,0) size 0x0
RenderInline {A} at (0,0) size 52x12 [color=#0000EE]
RenderText {TEXT} at (19,6) size 52x12
text run at (19,6) width 52: "Appetizers"
RenderListItem {LI} at (0,21) size 120x21
RenderListMarker at (5,6) size 0x10
RenderInline (generated) at (0,0) size 14x24
RenderText {TEXT} at (5,-4) size 14x10
text run at (5,-4) width 14: "\x{B7} "
RenderText {TEXT} at (0,0) size 0x0
RenderInline {A} at (0,0) size 78x12 [color=#0000EE]
RenderText {TEXT} at (19,6) size 78x12
text run at (19,6) width 78: "Soups & Salads"
RenderListItem {LI} at (0,42) size 120x33
RenderListMarker at (5,6) size 0x10
RenderInline (generated) at (0,0) size 14x24
RenderText {TEXT} at (5,-4) size 14x10
text run at (5,-4) width 14: "\x{B7} "
RenderText {TEXT} at (0,0) size 0x0
RenderInline {A} at (0,0) size 72x24 [color=#0000EE]
RenderText {TEXT} at (19,6) size 72x24
text run at (19,6) width 69: "Sandwiches &"
text run at (16,18) width 38: "Burgers"
RenderListItem {LI} at (0,75) size 120x21
RenderListMarker at (5,6) size 0x10
RenderInline (generated) at (0,0) size 14x24
RenderText {TEXT} at (5,-4) size 14x10
text run at (5,-4) width 14: "\x{B7} "
RenderText {TEXT} at (0,0) size 0x0
RenderInline {A} at (0,0) size 65x12 [color=#0000EE]
RenderText {TEXT} at (19,6) size 65x12
text run at (19,6) width 65: "Steak & Ribs"
RenderListItem {LI} at (0,96) size 120x21
RenderListMarker at (5,6) size 0x10
RenderInline (generated) at (0,0) size 14x24
RenderText {TEXT} at (5,-4) size 14x10
text run at (5,-4) width 14: "\x{B7} "
RenderText {TEXT} at (0,0) size 0x0
RenderInline {A} at (0,0) size 41x12 [color=#0000EE]
RenderText {TEXT} at (19,6) size 41x12
text run at (19,6) width 41: "Seafood"
RenderListItem {LI} at (0,117) size 120x21
RenderListMarker at (5,6) size 0x10
RenderInline (generated) at (0,0) size 14x24
RenderText {TEXT} at (5,-4) size 14x10
text run at (5,-4) width 14: "\x{B7} "
RenderText {TEXT} at (0,0) size 0x0
RenderInline {A} at (0,0) size 40x12 [color=#0000EE]
RenderText {TEXT} at (19,6) size 40x12
text run at (19,6) width 40: "Combos"
selection start: position 0 of child 0 {TEXT} of child 1 {A} of child 1 {LI} of child 1 {UL} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
selection end: position 5 of child 0 {TEXT} of child 1 {A} of child 7 {LI} of child 1 {UL} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
1.1 WebCore/layout-tests/editing/selection/extend-by-word-002.html
Index: extend-by-word-002.html
===================================================================
<html>
<head>
<style>
.editing {
border: 2px solid red;
padding: 12px;
font-size: 24px;
}
.cell {
padding: 12px;
font-size: 24px;
height: 48px;
}
li {
font-size: 12px;
font-family: Verdana, Arial, sans-serif;
}
div.nav {
width:120px;
}
ul.menu, ul.menu li {
margin: 0;padding:0;
font-size:10px
}
ul.menu li { padding: 3px; padding-left: 1.6em; padding-right:5px; text-indent: -1.1em !important; text-indent: -.5em; }
ul.menu li:first-letter { font-size:20px;line-height:10px; }
</style>
<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
<script>
function editingTest() {
for (i = 0; i < 6; i++)
extendSelectionForwardByWordCommand();
}
</script>
<title>Editing Test</title>
</head>
<body>
<div contenteditable id="root" class="editing" style="width:120px;">
<ul class="menu" id="test">
<li>· <a href="detail.asp?cat=7">Appetizers</a></li>
<li>· <a href="detail.asp?cat=13">Soups & Salads</a></li>
<li>· <a href="detail.asp?cat=5">Sandwiches & Burgers</a></li>
<li>· <a href="detail.asp?cat=14">Steak & Ribs</a></li>
<li>· <a href="detail.asp?cat=11">Seafood</a></li>
<li>· <a href="detail.asp?cat=17">Combos</a></li>
</ul>
</div>
<!--
Specifically checks test case in bug:
<rdar://problem/4244976> reproducible hang at ocharleys.com in VisiblePosition::initDownstream
-->
<script>
runEditingTest();
</script>
</body>
</html>
1.202 +69 -60 WebCore/khtml/rendering/render_block.cpp
Index: render_block.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/rendering/render_block.cpp,v
retrieving revision 1.201
retrieving revision 1.202
diff -u -r1.201 -r1.202
--- render_block.cpp 31 Aug 2005 05:12:54 -0000 1.201
+++ render_block.cpp 15 Sep 2005 23:50:17 -0000 1.202
@@ -3304,75 +3304,84 @@
currChild = currChild->firstChild();
// Get list markers out of the way.
- while (currChild && currChild->isListMarker()) currChild = currChild->nextSibling();
+ while (currChild && currChild->isListMarker())
+ currChild = currChild->nextSibling();
- if (currChild) {
- RenderObject* firstLetterContainer = currChild->parent();
- if (!firstLetterContainer)
- firstLetterContainer = this;
+ if (!currChild)
+ return;
- // If the child already has style, then it has already been created, so we just want
- // to update it.
- if (currChild->style()->styleType() == RenderStyle::FIRST_LETTER) {
- RenderStyle* pseudo = firstLetterBlock->getPseudoStyle(RenderStyle::FIRST_LETTER,
- firstLetterContainer->style(true));
- currChild->setStyle(pseudo);
- for (RenderObject* genChild = currChild->firstChild(); genChild; genChild = genChild->nextSibling())
- if (genChild->isText())
- genChild->setStyle(pseudo);
- return;
+ RenderObject* firstLetterContainer = currChild->parent();
+
+ // If the child already has style, then it has already been created, so we just want
+ // to update it.
+ if (currChild->style()->styleType() == RenderStyle::FIRST_LETTER) {
+ RenderStyle* pseudo = firstLetterBlock->getPseudoStyle(RenderStyle::FIRST_LETTER,
+ firstLetterContainer->style(true));
+ currChild->setStyle(pseudo);
+ for (RenderObject* genChild = currChild->firstChild(); genChild; genChild = genChild->nextSibling()) {
+ if (genChild->isText())
+ genChild->setStyle(pseudo);
}
-
- // If the child does not already have style, we create it here.
- if (currChild->isText() && !currChild->isBR() &&
- currChild->parent()->style()->styleType() != RenderStyle::FIRST_LETTER) {
-
- RenderText* textObj = static_cast<RenderText*>(currChild);
+ return;
+ }
+
+ // If the child does not already have style, we create it here.
+ if (currChild->isText() && !currChild->isBR() &&
+ currChild->parent()->style()->styleType() != RenderStyle::FIRST_LETTER) {
+
+ RenderText* textObj = static_cast<RenderText*>(currChild);
+
+ // Create our pseudo style now that we have our firstLetterContainer determined.
+ RenderStyle* pseudoStyle = firstLetterBlock->getPseudoStyle(RenderStyle::FIRST_LETTER,
+ firstLetterContainer->style(true));
+
+ // Force inline display (except for floating first-letters)
+ pseudoStyle->setDisplay( pseudoStyle->isFloating() ? BLOCK : INLINE);
+ pseudoStyle->setPosition( STATIC ); // CSS2 says first-letter can't be positioned.
+
+ RenderObject* firstLetter = RenderFlow::createAnonymousFlow(document(), pseudoStyle); // anonymous box
+ // FIXME: This adds in the wrong place if list markers were skipped above. Should be
+ // firstLetterContainer->addChild(firstLetter, currChild);
+ firstLetterContainer->addChild(firstLetter, firstLetterContainer->firstChild());
+
+ // The original string is going to be either a generated content string or a DOM node's
+ // string. We want the original string before it got transformed in case first-letter has
+ // no text-transform or a different text-transform applied to it.
+ SharedPtr<DOMStringImpl> oldText = textObj->originalString();
+ KHTMLAssert(oldText);
+
+ if (oldText.notNull() && oldText->l > 0) {
+ unsigned int length = 0;
- // Create our pseudo style now that we have our firstLetterContainer determined.
- RenderStyle* pseudoStyle = firstLetterBlock->getPseudoStyle(RenderStyle::FIRST_LETTER,
- firstLetterContainer->style(true));
+ // account for leading spaces and punctuation
+ while ( length < oldText->l && ( (oldText->s+length)->isSpace() || (oldText->s+length)->isPunct() ) )
+ length++;
- // Force inline display (except for floating first-letters)
- pseudoStyle->setDisplay( pseudoStyle->isFloating() ? BLOCK : INLINE);
- pseudoStyle->setPosition( STATIC ); // CSS2 says first-letter can't be positioned.
+ // account for first letter
+ length++;
+ //kdDebug( 6040 ) << "letter= '" << DOMString(oldText->substring(0,length)).qstring() << "'" << endl;
- RenderObject* firstLetter = RenderFlow::createAnonymousFlow(document(), pseudoStyle); // anonymous box
- firstLetterContainer->addChild(firstLetter, firstLetterContainer->firstChild());
+ // construct text fragment for the text after the first letter
+ // NOTE: this might empty
+ RenderTextFragment* remainingText =
+ new (renderArena()) RenderTextFragment(textObj->node(), oldText.get(), length, oldText->l-length);
+ remainingText->setStyle(textObj->style());
+ if (remainingText->element())
+ remainingText->element()->setRenderer(remainingText);
- // The original string is going to be either a generated content string or a DOM node's
- // string. We want the original string before it got transformed in case first-letter has
- // no text-transform or a different text-transform applied to it.
- SharedPtr<DOMStringImpl> oldText = textObj->originalString();
- KHTMLAssert(oldText);
+ RenderObject* nextObj = textObj->nextSibling();
+ firstLetterContainer->removeChild(textObj);
+ firstLetterContainer->addChild(remainingText, nextObj);
- if (oldText.notNull() && oldText->l >= 1) {
- unsigned int length = 0;
- while ( length < oldText->l &&
- ( (oldText->s+length)->isSpace() || (oldText->s+length)->isPunct() ) )
- length++;
- length++;
- //kdDebug( 6040 ) << "letter= '" << DOMString(oldText->substring(0,length)).qstring() << "'" << endl;
-
- RenderTextFragment* remainingText =
- new (renderArena()) RenderTextFragment(textObj->node(), oldText.get(), length, oldText->l-length);
- remainingText->setStyle(textObj->style());
- if (remainingText->element())
- remainingText->element()->setRenderer(remainingText);
-
- RenderObject* nextObj = textObj->nextSibling();
- firstLetterContainer->removeChild(textObj);
- firstLetterContainer->addChild(remainingText, nextObj);
-
- RenderTextFragment* letter =
- new (renderArena()) RenderTextFragment(remainingText->node(), oldText.get(), 0, length);
- RenderStyle* newStyle = new (renderArena()) RenderStyle();
- newStyle->inheritFrom(pseudoStyle);
- letter->setStyle(newStyle);
- firstLetter->addChild(letter);
+ // construct text fragment for the first letter
+ RenderTextFragment* letter =
+ new (renderArena()) RenderTextFragment(remainingText->node(), oldText.get(), 0, length);
+ RenderStyle* newStyle = new (renderArena()) RenderStyle();
+ newStyle->inheritFrom(pseudoStyle);
+ letter->setStyle(newStyle);
+ firstLetter->addChild(letter);
- textObj->detach();;
- }
+ textObj->detach();
}
}
}
1.197 +23 -15 WebCore/khtml/rendering/render_text.cpp
Index: render_text.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/rendering/render_text.cpp,v
retrieving revision 1.196
retrieving revision 1.197
diff -u -r1.196 -r1.197
--- render_text.cpp 4 Sep 2005 07:42:31 -0000 1.196
+++ render_text.cpp 15 Sep 2005 23:50:18 -0000 1.197
@@ -695,33 +695,41 @@
iterator = ubrk_open(UBRK_CHARACTER, "en_us", NULL, 0, &status);
createdIterator = true;
}
- if (!iterator) {
+ if (!iterator)
return NULL;
- }
+
status = U_ZERO_ERROR;
ubrk_setText(iterator, reinterpret_cast<const UChar *>(i->s), i->l, &status);
- if (status != U_ZERO_ERROR) {
+ if (status != U_ZERO_ERROR)
return NULL;
- }
+
return iterator;
}
long RenderText::previousOffset (long current) const
{
UBreakIterator *iterator = getCharacterBreakIterator(str);
- if (iterator) {
- return ubrk_preceding(iterator, current);
- }
- return current - 1;
+ if (!iterator)
+ return current - 1;
+
+ long result = ubrk_preceding(iterator, current);
+ if (result == UBRK_DONE)
+ result = current - 1;
+
+ return result;
}
long RenderText::nextOffset (long current) const
{
UBreakIterator *iterator = getCharacterBreakIterator(str);
- if (iterator) {
- return ubrk_following(iterator, current);
- }
- return current + 1;
+ if (!iterator)
+ return current + 1;
+
+ long result = ubrk_following(iterator, current);
+ if (result == UBRK_DONE)
+ result = current + 1;
+
+ return result;
}
int InlineTextBox::textPos() const
@@ -1860,9 +1868,9 @@
}
RenderTextFragment::RenderTextFragment(DOM::NodeImpl* _node, DOM::DOMStringImpl* _str,
- int startOffset, int endOffset)
-:RenderText(_node, _str->substring(startOffset, endOffset)),
-m_start(startOffset), m_end(endOffset), m_generatedContentStr(0)
+ int startOffset, int length)
+:RenderText(_node, _str->substring(startOffset, length)),
+m_start(startOffset), m_end(length), m_generatedContentStr(0)
{}
RenderTextFragment::RenderTextFragment(DOM::NodeImpl* _node, DOM::DOMStringImpl* _str)
1.88 +1 -1 WebCore/khtml/rendering/render_text.h
Index: render_text.h
===================================================================
RCS file: /cvs/root/WebCore/khtml/rendering/render_text.h,v
retrieving revision 1.87
retrieving revision 1.88
diff -u -r1.87 -r1.88
--- render_text.h 1 Sep 2005 16:21:05 -0000 1.87
+++ render_text.h 15 Sep 2005 23:50:19 -0000 1.88
@@ -320,7 +320,7 @@
{
public:
RenderTextFragment(DOM::NodeImpl* _node, DOM::DOMStringImpl* _str,
- int startOffset, int endOffset);
+ int startOffset, int length);
RenderTextFragment(DOM::NodeImpl* _node, DOM::DOMStringImpl* _str);
~RenderTextFragment();
More information about the webkit-changes
mailing list