WebCore:
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2008 22:59:29 +0000 (22:59 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2008 22:59:29 +0000 (22:59 +0000)
        Not reviewed: rolling out a previous patch.

        Fixed <rdar://problem/5695439> Crash during GCController destructor on
        quitting browser

        Used svn merge to roll out r29603 because it introduced some crashes
        on quit.

        GC relies on static hash tables, so it's not safe to GC from a static
        destructor, which might run after the static hash tables' destructors.

        * bindings/js/GCController.cpp:
        (WebCore::GCController::garbageCollectNow):
        * bindings/js/GCController.h:

LayoutTests:

        Fix for http://bugs.webkit.org/show_bug.cgi?id=15665

        Reviewed by Beth

        * fast/inline/inline-padding-disables-text-quirk.html: Added.
        * platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.checksum: Added.
        * platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.png: Added.
        * platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.txt: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/inline/inline-padding-disables-text-quirk.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/rendering/InlineFlowBox.cpp
WebCore/rendering/RenderObject.h
WebCore/rendering/bidi.cpp

index 4e8aadfbd7c267bd7fccba213ad880526e01bdff..3fbcaa1077e5d53019d4690771a155c2bdbae03a 100644 (file)
@@ -1,3 +1,14 @@
+2008-01-18  David Hyatt  <hyatt@apple.com>
+
+        Fix for http://bugs.webkit.org/show_bug.cgi?id=15665
+
+        Reviewed by Beth
+
+        * fast/inline/inline-padding-disables-text-quirk.html: Added.
+        * platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.checksum: Added.
+        * platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.png: Added.
+        * platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.txt: Added.
+
 2008-01-18  David Hyatt  <hyatt@apple.com>
 
         Updated results for http://bugs.webkit.org/show_bug.cgi?id=14975
diff --git a/LayoutTests/fast/inline/inline-padding-disables-text-quirk.html b/LayoutTests/fast/inline/inline-padding-disables-text-quirk.html
new file mode 100644 (file)
index 0000000..cf7dcbd
--- /dev/null
@@ -0,0 +1,7 @@
+<HTML> \r
+<BODY> \r
+<div style="background: green; border-bottom: 1px solid black;">\r
+<span style="padding: 0 0 0 1px;"></span></div> \r
+<div>There should be a green block above this text</div> \r
+ </BODY> \r
+</HTML> \r
diff --git a/LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.checksum b/LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.checksum
new file mode 100644 (file)
index 0000000..20c89b2
--- /dev/null
@@ -0,0 +1 @@
+5fee32327a27b0849c1c7261c9574f78
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.png b/LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.png
new file mode 100644 (file)
index 0000000..5ecf11d
Binary files /dev/null and b/LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.txt b/LayoutTests/platform/mac/fast/inline/inline-padding-disables-text-quirk-expected.txt
new file mode 100644 (file)
index 0000000..77d8175
--- /dev/null
@@ -0,0 +1,10 @@
+layer at (0,0) size 800x600
+  RenderView 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 784x19 [bgcolor=#008000] [border: (1px solid #000000) none]
+        RenderInline {SPAN} at (0,0) size 1x18
+      RenderBlock {DIV} at (0,19) size 784x18
+        RenderText {#text} at (0,0) size 286x18
+          text run at (0,0) width 286: "There should be a green block above this text"
index b71e97d87e992f413e0bcbd03a54eab5c726005b..a83c1bfbb195b3bd9c5319bc4b26e384f9a24a8f 100644 (file)
         (WebCore::GCController::garbageCollectNow):
         * bindings/js/GCController.h:
 
+2008-01-18  David Hyatt  <hyatt@apple.com>
+
+        Fix for http://bugs.webkit.org/show_bug.cgi?id=15665
+
+        Building on Beth's earlier work to start building line boxes for empty inlines, this patch makes more
+        empty inline cases work.  Empty inlines on lines by themselves now set isLineEmpty to false so that
+        bidiReorderLine will get properly called.  In addition, the "shrink boxes with no text children" quirk
+        needs to be disabled for inlines with padding, margins or borders.
+
+        Reviewed by Beth
+
+        Added fast/inline/inline-padding-disables-text-quirk.html
+
+        * rendering/InlineFlowBox.cpp:
+        (WebCore::InlineFlowBox::computeLogicalBoxHeights):
+        (WebCore::InlineFlowBox::placeBoxesVertically):
+        (WebCore::InlineFlowBox::shrinkBoxesWithNoTextChildren):
+        * rendering/RenderObject.h:
+        (WebCore::RenderObject::hasBordersPaddingOrMargin):
+        * rendering/bidi.cpp:
+        (WebCore::inlineFlowRequiresLineBox):
+        (WebCore::RenderBlock::findNextLineBreak):
+
 2008-01-18  David Hyatt  <hyatt@apple.com>
 
         Fix for http://bugs.webkit.org/show_bug.cgi?id=14975
index 57a875c51ffbc068e1cb72607786dc12e14a0938..f05b035f72f4da42b8a634a10509222759847207 100644 (file)
@@ -431,7 +431,7 @@ void InlineFlowBox::computeLogicalBoxHeights(int& maxPositionTop, int& maxPositi
         }
         else
             setBaseline(object()->baselinePosition(m_firstLine, true));
-        if (hasTextChildren() || strictMode) {
+        if (hasTextChildren() || object()->hasBordersPaddingOrMargin() || strictMode) {
             int ascent = baseline();
             int descent = height() - ascent;
             if (maxAscent < ascent)
@@ -456,7 +456,7 @@ void InlineFlowBox::computeLogicalBoxHeights(int& maxPositionTop, int& maxPositi
             if (maxPositionBottom < curr->height())
                 maxPositionBottom = curr->height();
         }
-        else if (curr->hasTextChildren() || strictMode) {
+        else if (curr->hasTextChildren() || curr->object()->hasBordersPaddingOrMargin() || strictMode) {
             int ascent = curr->baseline() - curr->yPos();
             int descent = curr->height() - ascent;
             if (maxAscent < ascent)
@@ -491,7 +491,7 @@ void InlineFlowBox::placeBoxesVertically(int y, int maxHeight, int maxAscent, bo
         else if (curr->yPos() == PositionBottom)
             curr->setYPos(y + maxHeight - curr->height());
         else {
-            if (!curr->hasTextChildren() && !strictMode)
+            if (!curr->hasTextChildren() && !curr->object()->hasBordersPaddingOrMargin() && !strictMode)
                 childAffectsTopBottomPos = false;
             curr->setYPos(curr->yPos() + y + maxAscent - curr->baseline());
         }
@@ -546,7 +546,7 @@ void InlineFlowBox::placeBoxesVertically(int y, int maxHeight, int maxAscent, bo
         setHeight(font.ascent() + font.descent());
         setYPos(yPos() + baseline() - font.ascent());
         setBaseline(font.ascent());
-        if (hasTextChildren() || strictMode) {
+        if (hasTextChildren() || object()->hasBordersPaddingOrMargin() || strictMode) {
             selectionTop = min(selectionTop, yPos());
             selectionBottom = max(selectionBottom, yPos() + height());
         }
@@ -565,7 +565,7 @@ void InlineFlowBox::shrinkBoxesWithNoTextChildren(int topPos, int bottomPos)
     }
 
     // See if we have text children. If not, then we need to shrink ourselves to fit on the line.
-    if (!hasTextChildren()) {
+    if (!hasTextChildren() && !object()->hasBordersPaddingOrMargin()) {
         if (yPos() < topPos)
             setYPos(topPos);
         if (yPos() + height() > bottomPos)
index 02ca4b069e37387afc113001d2258e7dec1fdf05..1e2d55e89f51c5869071c3db419d6c45d456d997 100644 (file)
@@ -324,6 +324,10 @@ public:
     bool hasBoxDecorations() const { return m_paintBackground; }
     bool mustRepaintBackgroundOrBorder() const;
 
+    bool hasBordersPaddingOrMargin() const { return borderLeft() != 0 || borderRight() != 0 || borderTop() != 0 || borderBottom() != 0 ||
+                                                    paddingLeft() != 0 || paddingRight() != 0 || paddingTop() != 0 || paddingBottom() != 0 ||
+                                                    marginLeft() != 0 || marginRight() != 0 || marginTop() != 0 || marginBottom() != 0; }
+
     bool needsLayout() const { return m_needsLayout || m_normalChildNeedsLayout || m_posChildNeedsLayout; }
     bool selfNeedsLayout() const { return m_needsLayout; }
     bool posChildNeedsLayout() const { return m_posChildNeedsLayout; }
index 56e63d828650d28afe731de3b1e24714d7fd800d..d9262286d9aa7ff2623b73445d9cf2e8ce18a5ba 100644 (file)
@@ -1247,19 +1247,9 @@ static inline bool shouldPreserveNewline(RenderObject* object)
 static bool inlineFlowRequiresLineBox(RenderObject* flow)
 {
     // FIXME: Right now, we only allow line boxes for inlines that are truly empty.
-    // We need to fix this, though, because at the very least, inlines with only text
-    // children that is all whitespace should should also have line boxes. 
-    if (!flow->isInlineFlow() || flow->firstChild())
-        return false;
-
-    bool hasPaddingOrMargin = !(flow->paddingLeft() == 0 && flow->paddingRight() == 0
-        && flow->paddingTop() == 0 && flow->paddingBottom() == 0 
-        && flow->marginLeft() == 0 && flow->marginRight() == 0
-        && flow->marginTop() == 0 && flow->marginBottom() == 0);
-    if (flow->hasBoxDecorations() || hasPaddingOrMargin)
-        return true;
-
-    return false;
+    // We need to fix this, though, because at the very least, inlines containing only
+    // ignorable whitespace should should also have line boxes. 
+    return flow->isInlineFlow() && !flow->firstChild() && flow->hasBordersPaddingOrMargin();
 }
 
 static inline bool requiresLineBox(BidiIterator& it)
@@ -1501,6 +1491,7 @@ BidiIterator RenderBlock::findNextLineBreak(BidiIterator &start, BidiState &bidi
             // If this object is at the start of the line, we need to behave like list markers and 
             // start ignoring spaces.
             if (inlineFlowRequiresLineBox(o)) {
+                isLineEmpty = false;
                 if (ignoringSpaces) {
                     trailingSpaceObject = 0;
                     addMidpoint(BidiIterator(0, o, 0)); // Stop ignoring spaces.