- Revision
- 130555
- Author
- [email protected]
- Date
- 2012-10-05 14:54:20 -0700 (Fri, 05 Oct 2012)
Log Message
Inline continuations create :after generated content on style recalcs
https://bugs.webkit.org/show_bug.cgi?id=93170
Patch by Takashi Sakamoto <[email protected]> on 2012-10-05
Reviewed by Abhishek Arya.
Source/WebCore:
The bug is caused by RenderInline::styleDidChange's setContinuation(0).
RenderObjectChildList uses continuation to know whether the given
renderer should have AFTER render object or not.
However, setContinuation(0) makes RenderObjectChildList to
misunderstand that all continuations are last continuation.
To avoid the misunderstanding, added a new flag to class
RenderObejctChildList to enable/disable updating :after content (and
also :before content).
Tests: fast/css-generated-content/after-with-inline-continuation.html
fast/css-generated-content/dynamic-apply-after-for-inline.html
* rendering/RenderInline.cpp:
(WebCore::RenderInline::styleDidChange):
Disable upating :after content for continuations which are not
the last one during setStyle just after setContinuation(0).
The setStyle invokes RenderInline::styleDidChange and also invokes
updateBeforeAfterContent via the styleDidChange. This means,
the last continuation's updateBeforeAfterContent is also invoked
after setContinuation(0). We have to update :after for the last
continuation.
* rendering/RenderObjectChildList.cpp:
(WebCore):
(WebCore::RenderObjectChildList::updateBeforeAfterContent):
If s_updateBeforeAfterContent is false, quickly exit
updateBeforeAfterContent.
* rendering/RenderObjectChildList.h:
(RenderObjectChildList):
Added a new flag s_enableUpdateBeforeAfterContent to enable/disable
updating :before or :after content.
LayoutTests:
* fast/css-generated-content/after-with-inline-continuation-expected.html: Added.
* fast/css-generated-content/after-with-inline-continuation.html: Added.
* fast/css-generated-content/dynamic-apply-after-for-inline-expected.html: Added.
* fast/css-generated-content/dynamic-apply-after-for-inline.html: Added.
* http/tests/misc/acid3-expected.txt:
The acid3-expected.txt has the duplicate RenderBlock (positioned).
Two 'layer at(638, 18) size 20x20, ... text run at (0,0) width 20: "X"'
exist. So did reset-results for acid3-expected.txt.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (130554 => 130555)
--- trunk/LayoutTests/ChangeLog 2012-10-05 21:51:17 UTC (rev 130554)
+++ trunk/LayoutTests/ChangeLog 2012-10-05 21:54:20 UTC (rev 130555)
@@ -1,3 +1,19 @@
+2012-10-05 Takashi Sakamoto <[email protected]>
+
+ Inline continuations create :after generated content on style recalcs
+ https://bugs.webkit.org/show_bug.cgi?id=93170
+
+ Reviewed by Abhishek Arya.
+
+ * fast/css-generated-content/after-with-inline-continuation-expected.html: Added.
+ * fast/css-generated-content/after-with-inline-continuation.html: Added.
+ * fast/css-generated-content/dynamic-apply-after-for-inline-expected.html: Added.
+ * fast/css-generated-content/dynamic-apply-after-for-inline.html: Added.
+ * http/tests/misc/acid3-expected.txt:
+ The acid3-expected.txt has the duplicate RenderBlock (positioned).
+ Two 'layer at(638, 18) size 20x20, ... text run at (0,0) width 20: "X"'
+ exist. So did reset-results for acid3-expected.txt.
+
2012-10-05 Tony Chang <[email protected]>
Fix margin box ascent computation in flexbox
Added: trunk/LayoutTests/fast/css-generated-content/after-with-inline-continuation-expected.html (0 => 130555)
--- trunk/LayoutTests/fast/css-generated-content/after-with-inline-continuation-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css-generated-content/after-with-inline-continuation-expected.html 2012-10-05 21:54:20 UTC (rev 130555)
@@ -0,0 +1,36 @@
+<!doctype html>
+<head>
+<style>
+ span {
+ display: inline;
+ }
+ .block {
+ font-family: Arial;
+ display: inline-block;
+ padding: 5px;
+ margin: 5px;
+ border: 1px solid red;
+ }
+</style>
+</head>
+<body>
+<div class="block">A</div>
+<span>
+ <div class="block">A</div>
+ <span>
+ <div class="block">A</div>
+ <span>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ </span>
+ <div class="block">B</div>
+ </span>
+ <div class="block">B</div>
+</span>
+<div class="block">B</div>
+</body>
Added: trunk/LayoutTests/fast/css-generated-content/after-with-inline-continuation.html (0 => 130555)
--- trunk/LayoutTests/fast/css-generated-content/after-with-inline-continuation.html (rev 0)
+++ trunk/LayoutTests/fast/css-generated-content/after-with-inline-continuation.html 2012-10-05 21:54:20 UTC (rev 130555)
@@ -0,0 +1,60 @@
+<!doctype html>
+<head>
+<style>
+ span {
+ /* Must be inline to cause a continuation chain */
+ display: inline;
+ }
+
+ span:before,
+ span:after {
+ /* Not requireed for the test, just makes it easier to see what's going on */
+ display: inline-block;
+ padding: 5px;
+ margin: 5px;
+ border: 1px solid red;
+ }
+
+ span:before {
+ content: "A";
+ }
+
+ span:after {
+ /* This content will get incorrectly repeated in the continuation */
+ content: "B";
+ }
+</style>
+</head>
+<script>
+if (window.testRunner)
+ testRunner.waitUntilDone();
+
+function runTest() {
+ setTimeout(function() {
+ // Cause a style recalc.
+ document.querySelector('span').style.fontFamily = 'Arial';
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 10);
+ };
+</script>
+
+<body _onload_="runTest()">
+
+<!-- [bug 93170] http://bugs.webkit.org/show_bug.cgi?id=91370 -->
+<!-- If test passes, you should see A A A B B B. -->
+
+<span>
+ <span>
+ <span>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ </span>
+ </span>
+</span>
+</body>
Added: trunk/LayoutTests/fast/css-generated-content/dynamic-apply-after-for-inline-expected.html (0 => 130555)
--- trunk/LayoutTests/fast/css-generated-content/dynamic-apply-after-for-inline-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css-generated-content/dynamic-apply-after-for-inline-expected.html 2012-10-05 21:54:20 UTC (rev 130555)
@@ -0,0 +1,19 @@
+<!doctype html>
+<html>
+<head>
+<style>
+ul {
+ display:inline;
+}
+ul::before {
+ content: 'before';
+}
+ul::after {
+ content: 'after';
+}
+</style>
+</head>
+<body>
+<ul><li>1</li><li>2</li><li>3</li></ul>
+</body>
+</html>
Added: trunk/LayoutTests/fast/css-generated-content/dynamic-apply-after-for-inline.html (0 => 130555)
--- trunk/LayoutTests/fast/css-generated-content/dynamic-apply-after-for-inline.html (rev 0)
+++ trunk/LayoutTests/fast/css-generated-content/dynamic-apply-after-for-inline.html 2012-10-05 21:54:20 UTC (rev 130555)
@@ -0,0 +1,48 @@
+<!doctype html>
+<html>
+<head>
+<style>
+ul {
+ display:inline;
+}
+ul.closed * {
+ display:none;
+}
+ul::before {
+ content: 'before';
+}
+ul::after {
+ content: 'after';
+}
+</style>
+<script>
+function toggle(ul) {
+ if (ul.className !== 'closed') {
+ ul.className = 'closed';
+ } else {
+ ul.className = '';
+ }
+ document.body.offsetLeft;
+}
+
+function runTest() {
+ var button = document.getElementById("toggle");
+ var ul = document.querySelectorAll('ul')[0];
+ toggle(ul);
+ toggle(ul);
+ toggle(ul);
+ toggle(ul);
+ toggle(ul);
+ toggle(ul);
+ toggle(ul);
+ toggle(ul);
+}
+</script>
+</head>
+<body _onload_="runTest()">
+<!-- [bug 93170] http://bugs.webkit.org/show_bug.cgi?id=91370 -->
+<!-- Dynamically apply / not apply :after to inline elements. -->
+<!-- If test passes, only 1 'before' and 1 'after' are shown. -->
+ <ul><li>1</li><li>2</li><li>3</li></ul>
+</body>
+</html>
Modified: trunk/LayoutTests/http/tests/misc/acid3-expected.txt (130554 => 130555)
--- trunk/LayoutTests/http/tests/misc/acid3-expected.txt 2012-10-05 21:51:17 UTC (rev 130554)
+++ trunk/LayoutTests/http/tests/misc/acid3-expected.txt 2012-10-05 21:54:20 UTC (rev 130555)
@@ -216,10 +216,6 @@
text run at (131,39) width 168: "this reference rendering"
RenderText {#text} at (299,39) size 4x17
text run at (299,39) width 4: "."
-layer at (638,18) size 20x20
- RenderBlock (positioned) at (638,18) size 20x20 [color=#FFFFFF] [bgcolor=#FF00FF]
- RenderText at (0,0) size 20x20
- text run at (0,0) width 20: "X"
layer at (130,84) size 300x150
RenderEmbeddedObject {OBJECT} at (130,84) size 300x150
layer at (0,0) size 300x150
Modified: trunk/Source/WebCore/ChangeLog (130554 => 130555)
--- trunk/Source/WebCore/ChangeLog 2012-10-05 21:51:17 UTC (rev 130554)
+++ trunk/Source/WebCore/ChangeLog 2012-10-05 21:54:20 UTC (rev 130555)
@@ -1,3 +1,41 @@
+2012-10-05 Takashi Sakamoto <[email protected]>
+
+ Inline continuations create :after generated content on style recalcs
+ https://bugs.webkit.org/show_bug.cgi?id=93170
+
+ Reviewed by Abhishek Arya.
+
+ The bug is caused by RenderInline::styleDidChange's setContinuation(0).
+ RenderObjectChildList uses continuation to know whether the given
+ renderer should have AFTER render object or not.
+ However, setContinuation(0) makes RenderObjectChildList to
+ misunderstand that all continuations are last continuation.
+ To avoid the misunderstanding, added a new flag to class
+ RenderObejctChildList to enable/disable updating :after content (and
+ also :before content).
+
+ Tests: fast/css-generated-content/after-with-inline-continuation.html
+ fast/css-generated-content/dynamic-apply-after-for-inline.html
+
+ * rendering/RenderInline.cpp:
+ (WebCore::RenderInline::styleDidChange):
+ Disable upating :after content for continuations which are not
+ the last one during setStyle just after setContinuation(0).
+ The setStyle invokes RenderInline::styleDidChange and also invokes
+ updateBeforeAfterContent via the styleDidChange. This means,
+ the last continuation's updateBeforeAfterContent is also invoked
+ after setContinuation(0). We have to update :after for the last
+ continuation.
+ * rendering/RenderObjectChildList.cpp:
+ (WebCore):
+ (WebCore::RenderObjectChildList::updateBeforeAfterContent):
+ If s_updateBeforeAfterContent is false, quickly exit
+ updateBeforeAfterContent.
+ * rendering/RenderObjectChildList.h:
+ (RenderObjectChildList):
+ Added a new flag s_enableUpdateBeforeAfterContent to enable/disable
+ updating :before or :after content.
+
2012-10-05 Simon Fraser <[email protected]>
Don't assume that TileCache layers are opaque
Modified: trunk/Source/WebCore/rendering/RenderInline.cpp (130554 => 130555)
--- trunk/Source/WebCore/rendering/RenderInline.cpp 2012-10-05 21:51:17 UTC (rev 130554)
+++ trunk/Source/WebCore/rendering/RenderInline.cpp 2012-10-05 21:54:20 UTC (rev 130555)
@@ -40,6 +40,8 @@
#include "TransformState.h"
#include "VisiblePosition.h"
+#include <wtf/TemporaryChange.h>
+
#if ENABLE(DASHBOARD_SUPPORT) || ENABLE(WIDGET_REGION)
#include "Frame.h"
#endif
@@ -167,11 +169,18 @@
// need to pass its style on to anyone else.
RenderStyle* newStyle = style();
RenderInline* continuation = inlineElementContinuation();
- for (RenderInline* currCont = continuation; currCont; currCont = currCont->inlineElementContinuation()) {
- RenderBoxModelObject* nextCont = currCont->continuation();
- currCont->setContinuation(0);
- currCont->setStyle(newStyle);
- currCont->setContinuation(nextCont);
+ {
+ TemporaryChange<bool> enableAfter(RenderObjectChildList::s_enableUpdateBeforeAfterContent, false);
+ RenderInline* nextInlineElementCont = 0;
+ for (RenderInline* currCont = continuation; currCont; currCont = nextInlineElementCont) {
+ nextInlineElementCont = currCont->inlineElementContinuation();
+ // We need to update :after content for the last continuation in the chain.
+ RenderObjectChildList::s_enableUpdateBeforeAfterContent = !nextInlineElementCont;
+ RenderBoxModelObject* nextCont = currCont->continuation();
+ currCont->setContinuation(0);
+ currCont->setStyle(newStyle);
+ currCont->setContinuation(nextCont);
+ }
}
// If an inline's in-flow positioning has changed then any descendant blocks will need to change their in-flow positioning accordingly.
Modified: trunk/Source/WebCore/rendering/RenderObjectChildList.cpp (130554 => 130555)
--- trunk/Source/WebCore/rendering/RenderObjectChildList.cpp 2012-10-05 21:51:17 UTC (rev 130554)
+++ trunk/Source/WebCore/rendering/RenderObjectChildList.cpp 2012-10-05 21:54:20 UTC (rev 130555)
@@ -45,6 +45,8 @@
namespace WebCore {
+bool RenderObjectChildList::s_enableUpdateBeforeAfterContent = true;
+
void RenderObjectChildList::destroyLeftoverChildren()
{
while (firstChild()) {
@@ -374,6 +376,8 @@
// In CSS2, before/after pseudo-content cannot nest. Check this first.
if (owner->style()->styleType() == BEFORE || owner->style()->styleType() == AFTER)
return;
+ if (!s_enableUpdateBeforeAfterContent)
+ return;
if (!styledObject)
styledObject = owner;
Modified: trunk/Source/WebCore/rendering/RenderObjectChildList.h (130554 => 130555)
--- trunk/Source/WebCore/rendering/RenderObjectChildList.h 2012-10-05 21:51:17 UTC (rev 130554)
+++ trunk/Source/WebCore/rendering/RenderObjectChildList.h 2012-10-05 21:54:20 UTC (rev 130555)
@@ -60,6 +60,9 @@
RenderObject* beforePseudoElementRenderer(const RenderObject* owner) const;
RenderObject* afterPseudoElementRenderer(const RenderObject* owner) const;
+public:
+ static bool s_enableUpdateBeforeAfterContent;
+
private:
void updateBeforeAfterStyle(RenderObject* child, PseudoId type, RenderStyle* pseudoElementStyle);
