2010-10-28 Yuzo Fujishima <yuzo@google.com>
authorinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Oct 2010 19:30:45 +0000 (19:30 +0000)
committerinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Oct 2010 19:30:45 +0000 (19:30 +0000)
        Reviewed by David Hyatt.

        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-28  Yuzo Fujishima  <yuzo@google.com>

        Reviewed by David Hyatt.

        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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@70797 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css/first-letter-nested-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/first-letter-nested.html [new file with mode: 0644]
LayoutTests/fast/css/first-letter-removed-added-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/first-letter-removed-added.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/rendering/RenderBlock.cpp
WebCore/rendering/RenderBlock.h
WebCore/rendering/RenderTextFragment.cpp
WebCore/rendering/RenderTextFragment.h
WebCore/rendering/style/RenderStyle.cpp
WebCore/rendering/style/RenderStyle.h

index 2e19864..d237b31 100644 (file)
@@ -1,3 +1,15 @@
+2010-10-28  Yuzo Fujishima  <yuzo@google.com>
+
+        Reviewed by David Hyatt.
+
+        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-28  Adrienne Walker  <enne@google.com>
 
         Reviewed by Kenneth Russell.
diff --git a/LayoutTests/fast/css/first-letter-nested-expected.txt b/LayoutTests/fast/css/first-letter-nested-expected.txt
new file mode 100644 (file)
index 0000000..77cf573
--- /dev/null
@@ -0,0 +1,16 @@
+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
+
diff --git a/LayoutTests/fast/css/first-letter-nested.html b/LayoutTests/fast/css/first-letter-nested.html
new file mode 100644 (file)
index 0000000..f58f4f0
--- /dev/null
@@ -0,0 +1,43 @@
+<!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>
diff --git a/LayoutTests/fast/css/first-letter-removed-added-expected.txt b/LayoutTests/fast/css/first-letter-removed-added-expected.txt
new file mode 100644 (file)
index 0000000..1f84e32
--- /dev/null
@@ -0,0 +1,42 @@
+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
+
diff --git a/LayoutTests/fast/css/first-letter-removed-added.html b/LayoutTests/fast/css/first-letter-removed-added.html
new file mode 100644 (file)
index 0000000..2f0f28d
--- /dev/null
@@ -0,0 +1,88 @@
+<!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 "; }
+</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>
index 16326a5..a1b75c6 100644 (file)
@@ -1,3 +1,31 @@
+2010-10-28  Yuzo Fujishima  <yuzo@google.com>
+
+        Reviewed by David Hyatt.
+
+        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-28  Benoit Jacob  <bjacob@mozilla.com>
 
         Reviewed by Kenneth Russell.
index 6a8c7e4..47ec220 100644 (file)
@@ -256,7 +256,6 @@ void RenderBlock::styleDidChange(StyleDifference diff, const RenderStyle* oldSty
         updateBeforeAfterContent(BEFORE);
         updateBeforeAfterContent(AFTER);
     }
-    updateFirstLetter();
 }
 
 void RenderBlock::updateBeforeAfterContent(PseudoId pseudoId)
index 66c8659..4956fc0 100644 (file)
@@ -181,6 +181,8 @@ public:
     int collapsedMarginBeforeForChild(RenderBox* child) const;
     int collapsedMarginAfterForChild(RenderBox* child) const;
 
+    virtual void updateFirstLetter();
+
     class MarginValues {
     public:
         MarginValues(int beforePos, int beforeNeg, int afterPos, int afterNeg)
@@ -269,8 +271,6 @@ protected:
     virtual int firstLineBoxBaseline() const;
     virtual int lastLineBoxBaseline() const;
 
-    virtual void updateFirstLetter();
-
     virtual void updateHitTestResult(HitTestResult&, const IntPoint&);
 
     // Delay update scrollbar until finishDelayRepaint() will be
index 705c095..ec62d85 100644 (file)
@@ -23,6 +23,7 @@
 #include "config.h"
 #include "RenderTextFragment.h"
 
+#include "RenderBlock.h"
 #include "Text.h"
 
 namespace WebCore {
@@ -57,6 +58,16 @@ PassRefPtr<StringImpl> RenderTextFragment::originalText() const
     return result->substring(start(), end());
 }
 
+void RenderTextFragment::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
+{
+    RenderText::styleDidChange(diff, oldStyle);
+
+    if (RenderBlock* block = blockForAccompanyingFirstLetter()) {
+        block->style()->removeCachedPseudoStyle(FIRST_LETTER);
+        block->updateFirstLetter();
+    }
+}
+
 void RenderTextFragment::destroy()
 {
     if (m_firstLetter)
@@ -92,4 +103,15 @@ UChar RenderTextFragment::previousCharacter() const
     return RenderText::previousCharacter();
 }
 
+RenderBlock* RenderTextFragment::blockForAccompanyingFirstLetter() const
+{
+    if (!m_firstLetter)
+        return 0;
+    for (RenderObject* block = m_firstLetter->parent(); block; block = block->parent()) {
+        if (block->style()->hasPseudoStyle(FIRST_LETTER) && block->canHaveChildren() && block->isRenderBlock())
+            return toRenderBlock(block);
+    }
+    return 0;
+}
+
 } // namespace WebCore
index abb3fa4..c251f81 100644 (file)
@@ -50,9 +50,13 @@ public:
     StringImpl* contentString() const { return m_contentString.get(); }
     virtual PassRefPtr<StringImpl> originalText() const;
 
+protected:
+    virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
+
 private:
     virtual void setTextInternal(PassRefPtr<StringImpl>);
     virtual UChar previousCharacter() const;
+    RenderBlock* blockForAccompanyingFirstLetter() const;
 
     unsigned m_start;
     unsigned m_end;
index 68c9186..ec77367 100644 (file)
@@ -245,6 +245,19 @@ RenderStyle* RenderStyle::addCachedPseudoStyle(PassRefPtr<RenderStyle> pseudo)
     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 ||
index d01425a..86a5574 100644 (file)
@@ -325,6 +325,7 @@ public:
 
     RenderStyle* getCachedPseudoStyle(PseudoId) const;
     RenderStyle* addCachedPseudoStyle(PassRefPtr<RenderStyle>);
+    void removeCachedPseudoStyle(PseudoId);
 
     const PseudoStyleCache* cachedPseudoStyles() const { return m_cachedPseudoStyles.get(); }