Attachment #71022 for bug #14550

View Details Raw Unified Return to Bug 14550

| Differences between



2010-10-18  Yuzo Fujishima  <yuzo@google.com>

        Reviewed by NOBODY (OOPS!).

        Fix for Bug 14550 - Non-layout style change does not update nested first-letter
        https://bugs.webkit.org/show_bug.cgi?id=14550

        * fast/css/first-letter-nested-expected.txt: Added.
        * fast/css/first-letter-nested.html: Added.
        * fast/css/first-letter-removed-added-expected.txt: Added.
        * fast/css/first-letter-removed-added.html: Added.

2010-10-17  Oliver Hunt  <oliver@apple.com>

        Reviewed by Sam Weinig.



The three spans below should look the same.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

Span

Span

Span

PASS document.getElementById('test_span_1').offsetWidth is document.getElementById('ref_span').offsetWidth
PASS document.getElementById('test_span_2').offsetWidth is document.getElementById('ref_span').offsetWidth
PASS successfullyParsed is true

TEST COMPLETE




<!doctype html>
<html>
<head>
<title>Test for first-letter for nested elements</title>
<style>
.test:first-letter { color: green; }
.boxed { border: solid 1px; }
</style>
<link rel="stylesheet" href="../js/resources/js-test-style.css"/>
<script src="../js/resources/js-test-pre.js"></script>
</head>
<body>
<div id="description"></div>

<p class="test" id="test_p_1">
    <span class="boxed" id="test_span_1">Span</span>
</p>
<p class="test">
    <span class="boxed" id="test_span_2" class="boxed">Span</span>
</p>
<p>
    <span class="boxed" id="ref_span" style="font-size:32px"><span style="color:green">S</span>pan</span>
</p>

<div id="console"></div>

<script>
document.body.offsetTop;

document.getElementById('test_p_1').style.fontSize = '32px';
document.getElementById('test_span_2').style.fontSize = '32px';

document.body.offsetTop;

description("The three spans below should look the same.");
shouldBe("document.getElementById('test_span_1').offsetWidth", "document.getElementById('ref_span').offsetWidth");
shouldBe("document.getElementById('test_span_2').offsetWidth", "document.getElementById('ref_span').offsetWidth");

var successfullyParsed = true;
</script>
<script src="../js/resources/js-test-post.js"></script>
</body>
</html>



The following pairs should look the same.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

Test 1:
Block to Inline
Block to Inline
Test 2:
Inline to Block
Inline to Block
Test 3:
pseudo
pseudo
Test 4: (Currently failing)
pseudo
pseudo
Test 5:
To Float
To Float

Test 6:
To Non Float
To Non Float
Test 7: (Currently failing)
To Float
To Float

Test 8:
To Non Float
To Non Float
PASS document.getElementById('test1').offsetWidth == document.getElementById('ref1').offsetWidth is true
PASS document.getElementById('test2').offsetHeight == document.getElementById('ref2').offsetHeight is true
PASS document.getElementById('test3').offsetWidth == document.getElementById('ref3').offsetWidth is true
FAIL document.getElementById('test4').offsetWidth == document.getElementById('ref4').offsetWidth should be true. Was false.
PASS document.getElementById('test5').offsetWidth == document.getElementById('ref5').offsetWidth is true
PASS document.getElementById('test6').offsetWidth == document.getElementById('ref6').offsetWidth is true
FAIL document.getElementById('test7').offsetWidth == document.getElementById('ref7').offsetWidth should be true. Was false.
PASS document.getElementById('test8').offsetWidth == document.getElementById('ref8').offsetWidth is true
PASS successfullyParsed is true

TEST COMPLETE




<!doctype html>
<html>
<head>
<title>Test for first-letter that is removed or added by style change</title>
<style>
    .test:first-letter { color: green; font-size:64px; float:inherit; }
    .test { clear:both; }
    .boxed { border: solid 1px; }
    .bef:before { content:"Before "; }
    .test_b:first-letter { color: green; font-size:64px; float:left; }
    .ref_b:first-letter { color: green; font-size:64px; float:none; }
</style>
<link rel="stylesheet" href="../js/resources/js-test-style.css"/>
<script src="../js/resources/js-test-pre.js"></script>
</head>
<body>
<div id="description"></div>

Test 1:<br/>
<div class="test boxed" id="test1">Block to Inline</div><br/>
<div class="test boxed" id="ref1" style="display:inline">Block to Inline</div><br/>
<hr/>

Test 2:<br/>
<span class="test boxed" id="test2">Inline to Block</span>
<span class="test boxed" id="ref2" style="display:block">Inline to Block</span>
<hr/>

Test 3:<br/>
<div class="test"><span id="test3" class="boxed bef">pseudo</span></div>
<div class="test"><span id="ref3" class="boxed">pseudo</span></div>
<hr/>

Test 4: (Currently failing)<br/>
<div class="test"><span id="test4" class="boxed">pseudo</span></div>
<div class="test"><span id="ref4" class="boxed bef">pseudo</span></div>
<hr/>

Test 5:<br/>
<div class="test"><span id="test5" class="boxed" style="float:none">To Float</span></div>
<div class="test"><span id="ref5" class="boxed" style="float:left">To Float</span></div>
<br/>
<hr/>

Test 6:<br/>
<div class="test"><span id="test6" class="boxed" style="float:left">To Non Float</span></div>
<div class="test"><span id="ref6" class="boxed" style="float:none">To Non Float</span></div>
<hr/>

Test 7: (Currently failing)<br/>
<div class="test"><div id="test7" class="boxed" style="float:none">To Float</div></div>
<div class="test"><div id="ref7" class="boxed" style="float:left">To Float</div></div>
<br/>
<hr/>

Test 8:<br/>
<div class="test"><div id="test8" class="boxed" style="float:left">To Non Float</div></div>
<div class="test"><div id="ref8" class="boxed" style="float:none">To Non Float</div></div>
<hr/>

<div id="console"></div>

<script>
description("The following pairs should look the same.");

document.body.offsetTop; // Force layout

document.getElementById("test1").style.display = "inline";
document.getElementById("test2").style.display = "block";
document.getElementById("test3").className = "boxed";
document.getElementById("test4").className = "boxed bef";
document.getElementById("test5").style.float = "left";
document.getElementById("test6").style.float = "none";
document.getElementById("test7").style.float = "left";
document.getElementById("test8").style.float = "none";

shouldBeTrue("document.getElementById('test1').offsetWidth == document.getElementById('ref1').offsetWidth");
shouldBeTrue("document.getElementById('test2').offsetHeight == document.getElementById('ref2').offsetHeight");
shouldBeTrue("document.getElementById('test3').offsetWidth == document.getElementById('ref3').offsetWidth");
shouldBeTrue("document.getElementById('test4').offsetWidth == document.getElementById('ref4').offsetWidth");
shouldBeTrue("document.getElementById('test5').offsetWidth == document.getElementById('ref5').offsetWidth");
shouldBeTrue("document.getElementById('test6').offsetWidth == document.getElementById('ref6').offsetWidth");
shouldBeTrue("document.getElementById('test7').offsetWidth == document.getElementById('ref7').offsetWidth");
shouldBeTrue("document.getElementById('test8').offsetWidth == document.getElementById('ref8').offsetWidth");

var successfullyParsed = true;
</script>
<script src="../js/resources/js-test-post.js"></script>
</body>
</html>



2010-10-18  Yuzo Fujishima  <yuzo@google.com>

        Reviewed by NOBODY (OOPS!).

        Fix for Bug 14550 - Non-layout style change does not update nested first-letter
        https://bugs.webkit.org/show_bug.cgi?id=14550

        If a render text fragment is accompanied by a first letter, update the
        first letter's style when the fragment's style is changed.

        Test: fast/css/first-letter-nested.html

        * rendering/RenderBlock.cpp:
        (WebCore::RenderBlock::styleDidChange): Stop calling updateFirstLetter
        from here.
        * rendering/RenderBlock.h: Make updateFirstLetter accessbile from
        RenderTextFragment.
        * rendering/RenderTextFragment.cpp:
        (WebCore::RenderTextFragment::styleDidChange): If appropriate, update
        first letter after removing stale cached pseudo style.
        (WebCore::RenderTextFragment::blockForAccompanyingFirstLetter): Helper
        to get the block for the first letter.
        * rendering/RenderTextFragment.h:
        * rendering/style/RenderStyle.cpp:
        (WebCore::RenderStyle::removeCachedPseudoStyle): Remove the specified
        pseudo style from cache.
        * rendering/style/RenderStyle.h:

2010-10-17  Adam Barth  <abarth@webkit.org>

        Reviewed by Dimitri Glazkov.

Lines 256-262 void RenderBlock::styleDidChange(StyleDifference diff, const RenderStyle* oldSty a/WebCore/rendering/RenderBlock.cpp_sec1
- a/WebCore/rendering/RenderBlock.cpp -1 lines
256	updateBeforeAfterContent(BEFORE);	256	updateBeforeAfterContent(BEFORE);
257	updateBeforeAfterContent(AFTER);	257	updateBeforeAfterContent(AFTER);
258	}	258	}
259	updateFirstLetter();
260	}	259	}
261		260
262	void RenderBlock::updateBeforeAfterContent(PseudoId pseudoId)	261	void RenderBlock::updateBeforeAfterContent(PseudoId pseudoId)

Lines 181-186 public: a/WebCore/rendering/RenderBlock.h_sec1
- a/WebCore/rendering/RenderBlock.h -2 / +2 lines
181	int collapsedMarginBeforeForChild(RenderBox* child) const;	181	int collapsedMarginBeforeForChild(RenderBox* child) const;
182	int collapsedMarginAfterForChild(RenderBox* child) const;	182	int collapsedMarginAfterForChild(RenderBox* child) const;
183		183
		184	virtual void updateFirstLetter();
		185
184	class MarginValues {	186	class MarginValues {
185	public:	187	public:
186	MarginValues(int beforePos, int beforeNeg, int afterPos, int afterNeg)	188	MarginValues(int beforePos, int beforeNeg, int afterPos, int afterNeg)
Lines 269-276 protected: a/WebCore/rendering/RenderBlock.h_sec2
269	virtual int firstLineBoxBaseline() const;	271	virtual int firstLineBoxBaseline() const;
270	virtual int lastLineBoxBaseline() const;	272	virtual int lastLineBoxBaseline() const;
271		273
272	virtual void updateFirstLetter();
273
274	virtual void updateHitTestResult(HitTestResult&, const IntPoint&);	274	virtual void updateHitTestResult(HitTestResult&, const IntPoint&);
275		275
276	// Delay update scrollbar until finishDelayRepaint() will be	276	// Delay update scrollbar until finishDelayRepaint() will be

Lines 23-28 a/WebCore/rendering/RenderTextFragment.cpp_sec1
- a/WebCore/rendering/RenderTextFragment.cpp +22 lines
23	#include "config.h"	23	#include "config.h"
24	#include "RenderTextFragment.h"	24	#include "RenderTextFragment.h"
25		25
		26	#include "RenderBlock.h"
26	#include "Text.h"	27	#include "Text.h"
27		28
28	namespace WebCore {	29	namespace WebCore {
Lines 57-62 PassRefPtr<StringImpl> RenderTextFragment::originalText() const a/WebCore/rendering/RenderTextFragment.cpp_sec2
57	return result->substring(start(), end());	58	return result->substring(start(), end());
58	}	59	}
59		60
		61	void RenderTextFragment::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
		62	{
		63	RenderText::styleDidChange(diff, oldStyle);
		64
		65	if (RenderBlock* block = blockForAccompanyingFirstLetter()) {
		66	block->style()->removeCachedPseudoStyle(FIRST_LETTER);
		67	block->updateFirstLetter();
		68	}
		69	}
		70
60	void RenderTextFragment::destroy()	71	void RenderTextFragment::destroy()
61	{	72	{
62	if (m_firstLetter)	73	if (m_firstLetter)
Lines 92-95 UChar RenderTextFragment::previousCharacter() const a/WebCore/rendering/RenderTextFragment.cpp_sec3
92	return RenderText::previousCharacter();	103	return RenderText::previousCharacter();
93	}	104	}
94		105
		106	RenderBlock* RenderTextFragment::blockForAccompanyingFirstLetter() const
		107	{
		108	if (!m_firstLetter)
		109	return 0;
		110	for (RenderObject* block = m_firstLetter->parent(); block; block = block->parent()) {
		111	if (block->style()->hasPseudoStyle(FIRST_LETTER) && block->canHaveChildren() && block->isRenderBlock())
		112	return toRenderBlock(block);
		113	}
		114	return 0;
		115	}
		116
95	} // namespace WebCore	117	} // namespace WebCore

Lines 50-58 public: a/WebCore/rendering/RenderTextFragment.h_sec1
- a/WebCore/rendering/RenderTextFragment.h +4 lines
50	StringImpl* contentString() const { return m_contentString.get(); }	50	StringImpl* contentString() const { return m_contentString.get(); }
51	virtual PassRefPtr<StringImpl> originalText() const;	51	virtual PassRefPtr<StringImpl> originalText() const;
52		52
		53	protected:
		54	virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
		55
53	private:	56	private:
54	virtual void setTextInternal(PassRefPtr<StringImpl>);	57	virtual void setTextInternal(PassRefPtr<StringImpl>);
55	virtual UChar previousCharacter() const;	58	virtual UChar previousCharacter() const;
		59	RenderBlock* blockForAccompanyingFirstLetter() const;
56		60
57	unsigned m_start;	61	unsigned m_start;
58	unsigned m_end;	62	unsigned m_end;



    return result;
}

void RenderStyle::removeCachedPseudoStyle(PseudoId pid)
{
    if (!m_cachedPseudoStyles)
        return;
    for (size_t i = 0; i < m_cachedPseudoStyles->size(); ++i) {
        RenderStyle* pseudoStyle = m_cachedPseudoStyles->at(i).get();
        if (pseudoStyle->styleType() == pid) {
            m_cachedPseudoStyles->remove(i);
            return;
        }
    }
}

bool RenderStyle::inheritedNotEqual(const RenderStyle* other) const
{
    return inherited_flags != other->inherited_flags ||

Lines 325-330 public: a/WebCore/rendering/style/RenderStyle.h_sec1
- a/WebCore/rendering/style/RenderStyle.h +1 lines
325		325
326	RenderStyle* getCachedPseudoStyle(PseudoId) const;	326	RenderStyle* getCachedPseudoStyle(PseudoId) const;
327	RenderStyle* addCachedPseudoStyle(PassRefPtr<RenderStyle>);	327	RenderStyle* addCachedPseudoStyle(PassRefPtr<RenderStyle>);
		328	void removeCachedPseudoStyle(PseudoId);
328		329
329	const PseudoStyleCache* cachedPseudoStyles() const { return m_cachedPseudoStyles.get(); }	330	const PseudoStyleCache* cachedPseudoStyles() const { return m_cachedPseudoStyles.get(); }
330		331

Return to Bug 14550

Lines 1-3 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +12 lines
		1	2010-10-18 Yuzo Fujishima <yuzo@google.com>
		2
		3	Reviewed by NOBODY (OOPS!).
		4
		5	Fix for Bug 14550 - Non-layout style change does not update nested first-letter
		6	https://bugs.webkit.org/show_bug.cgi?id=14550
		7
		8	* fast/css/first-letter-nested-expected.txt: Added.
		9	* fast/css/first-letter-nested.html: Added.
		10	* fast/css/first-letter-removed-added-expected.txt: Added.
		11	* fast/css/first-letter-removed-added.html: Added.
		12
1	2010-10-17 Oliver Hunt <oliver@apple.com>	13	2010-10-17 Oliver Hunt <oliver@apple.com>
2		14
3	Reviewed by Sam Weinig.	15	Reviewed by Sam Weinig.

Line 0 a/LayoutTests/fast/css/first-letter-nested-expected.txt_sec1
- a/LayoutTests/fast/css/first-letter-nested-expected.txt +16 lines
	1	The three spans below should look the same.
	2
	3	On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
	4
	5	Span
	6
	7	Span
	8
	9	Span
	10
	11	PASS document.getElementById('test_span_1').offsetWidth is document.getElementById('ref_span').offsetWidth
	12	PASS document.getElementById('test_span_2').offsetWidth is document.getElementById('ref_span').offsetWidth
	13	PASS successfullyParsed is true
	14
	15	TEST COMPLETE
	16

Line 0 a/LayoutTests/fast/css/first-letter-nested.html_sec1
- a/LayoutTests/fast/css/first-letter-nested.html +43 lines
	1	<!doctype html>
	2	<html>
	3	<head>
	4	<title>Test for first-letter for nested elements</title>
	5	<style>
	6	.test:first-letter { color: green; }
	7	.boxed { border: solid 1px; }
	8	</style>
	9	<link rel="stylesheet" href="../js/resources/js-test-style.css"/>
	10	<script src="../js/resources/js-test-pre.js"></script>
	11	</head>
	12	<body>
	13	<div id="description"></div>
	14
	15	<p class="test" id="test_p_1">
	16	<span class="boxed" id="test_span_1">Span</span>
	17	</p>
	18	<p class="test">
	19	<span class="boxed" id="test_span_2" class="boxed">Span</span>
	20	</p>
	21	<p>
	22	<span class="boxed" id="ref_span" style="font-size:32px"><span style="color:green">S</span>pan</span>
	23	</p>
	24
	25	<div id="console"></div>
	26
	27	<script>
	28	document.body.offsetTop;
	29
	30	document.getElementById('test_p_1').style.fontSize = '32px';
	31	document.getElementById('test_span_2').style.fontSize = '32px';
	32
	33	document.body.offsetTop;
	34
	35	description("The three spans below should look the same.");
	36	shouldBe("document.getElementById('test_span_1').offsetWidth", "document.getElementById('ref_span').offsetWidth");
	37	shouldBe("document.getElementById('test_span_2').offsetWidth", "document.getElementById('ref_span').offsetWidth");
	38
	39	var successfullyParsed = true;
	40	</script>
	41	<script src="../js/resources/js-test-post.js"></script>
	42	</body>
	43	</html>

Line 0 a/LayoutTests/fast/css/first-letter-removed-added-expected.txt_sec1
- a/LayoutTests/fast/css/first-letter-removed-added-expected.txt +42 lines
	1	The following pairs should look the same.
	2
	3	On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
	4
	5	Test 1:
	6	Block to Inline
	7	Block to Inline
	8	Test 2:
	9	Inline to Block
	10	Inline to Block
	11	Test 3:
	12	pseudo
	13	pseudo
	14	Test 4: (Currently failing)
	15	pseudo
	16	pseudo
	17	Test 5:
	18	To Float
	19	To Float
	20
	21	Test 6:
	22	To Non Float
	23	To Non Float
	24	Test 7: (Currently failing)
	25	To Float
	26	To Float
	27
	28	Test 8:
	29	To Non Float
	30	To Non Float
	31	PASS document.getElementById('test1').offsetWidth == document.getElementById('ref1').offsetWidth is true
	32	PASS document.getElementById('test2').offsetHeight == document.getElementById('ref2').offsetHeight is true
	33	PASS document.getElementById('test3').offsetWidth == document.getElementById('ref3').offsetWidth is true
	34	FAIL document.getElementById('test4').offsetWidth == document.getElementById('ref4').offsetWidth should be true. Was false.
	35	PASS document.getElementById('test5').offsetWidth == document.getElementById('ref5').offsetWidth is true
	36	PASS document.getElementById('test6').offsetWidth == document.getElementById('ref6').offsetWidth is true
	37	FAIL document.getElementById('test7').offsetWidth == document.getElementById('ref7').offsetWidth should be true. Was false.
	38	PASS document.getElementById('test8').offsetWidth == document.getElementById('ref8').offsetWidth is true
	39	PASS successfullyParsed is true
	40
	41	TEST COMPLETE
	42

Lines 1-3 a/WebCore/ChangeLog_sec1
- a/WebCore/ChangeLog +28 lines
		1	2010-10-18 Yuzo Fujishima <yuzo@google.com>
		2
		3	Reviewed by NOBODY (OOPS!).
		4
		5	Fix for Bug 14550 - Non-layout style change does not update nested first-letter
		6	https://bugs.webkit.org/show_bug.cgi?id=14550
		7
		8	If a render text fragment is accompanied by a first letter, update the
		9	first letter's style when the fragment's style is changed.
		10
		11	Test: fast/css/first-letter-nested.html
		12
		13	* rendering/RenderBlock.cpp:
		14	(WebCore::RenderBlock::styleDidChange): Stop calling updateFirstLetter
		15	from here.
		16	* rendering/RenderBlock.h: Make updateFirstLetter accessbile from
		17	RenderTextFragment.
		18	* rendering/RenderTextFragment.cpp:
		19	(WebCore::RenderTextFragment::styleDidChange): If appropriate, update
		20	first letter after removing stale cached pseudo style.
		21	(WebCore::RenderTextFragment::blockForAccompanyingFirstLetter): Helper
		22	to get the block for the first letter.
		23	* rendering/RenderTextFragment.h:
		24	* rendering/style/RenderStyle.cpp:
		25	(WebCore::RenderStyle::removeCachedPseudoStyle): Remove the specified
		26	pseudo style from cache.
		27	* rendering/style/RenderStyle.h:
		28
1	2010-10-17 Adam Barth <abarth@webkit.org>	29	2010-10-17 Adam Barth <abarth@webkit.org>
2		30
3	Reviewed by Dimitri Glazkov.	31	Reviewed by Dimitri Glazkov.

Lines 244-249 RenderStyle* RenderStyle::addCachedPseudoStyle(PassRefPtr<RenderStyle> pseudo) a/WebCore/rendering/style/RenderStyle.cpp_sec1
- a/WebCore/rendering/style/RenderStyle.cpp +13 lines
244	return result;	244	return result;
245	}	245	}
246		246
		247	void RenderStyle::removeCachedPseudoStyle(PseudoId pid)
		248	{
		249	if (!m_cachedPseudoStyles)
		250	return;
		251	for (size_t i = 0; i < m_cachedPseudoStyles->size(); ++i) {
		252	RenderStyle* pseudoStyle = m_cachedPseudoStyles->at(i).get();
		253	if (pseudoStyle->styleType() == pid) {
		254	m_cachedPseudoStyles->remove(i);
		255	return;
		256	}
		257	}
		258	}
		259
247	bool RenderStyle::inheritedNotEqual(const RenderStyle* other) const	260	bool RenderStyle::inheritedNotEqual(const RenderStyle* other) const
248	{	261	{
249	return inherited_flags != other->inherited_flags \|\|	262	return inherited_flags != other->inherited_flags \|\|