Commit fb1a91a8 authored by harrison's avatar harrison
Browse files

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.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@10541 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent e14e6483
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
<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>&middot; <a href="detail.asp?cat=7">Appetizers</a></li>
<li>&middot; <a href="detail.asp?cat=13">Soups & Salads</a></li>
<li>&middot; <a href="detail.asp?cat=5">Sandwiches & Burgers</a></li>
<li>&middot; <a href="detail.asp?cat=14">Steak & Ribs</a></li>
<li>&middot; <a href="detail.asp?cat=11">Seafood</a></li>
<li>&middot; <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>
2005-09-15 David Harrison <harrison@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@nypop.com>
Reviewed, tweaked, and landed by Darin.
......
......@@ -3304,75 +3304,84 @@ void RenderBlock::updateFirstLetter()
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);
textObj->detach();;
}
// 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();
}
}
}
......
......@@ -695,33 +695,41 @@ static UBreakIterator *getCharacterBreakIterator(const DOMStringImpl *i)
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 @@ InlineBox *RenderText::inlineBox(long offset, EAffinity affinity)
}
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)
......
......@@ -320,7 +320,7 @@ class RenderTextFragment : public RenderText
{
public:
RenderTextFragment(DOM::NodeImpl* _node, DOM::DOMStringImpl* _str,
int startOffset, int endOffset);
int startOffset, int length);
RenderTextFragment(DOM::NodeImpl* _node, DOM::DOMStringImpl* _str);
~RenderTextFragment();
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment