Bug 131366 - Padding-bottom and padding-top not applied to inline elements
Summary: Padding-bottom and padding-top not applied to inline elements
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml renderer (show other bugs)
Version: unspecified
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-07-25 22:10 UTC by Brandon Snider
Modified: 2006-08-23 04:50 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
testcase.html (778 bytes, text/html)
2006-07-25 22:48 UTC, Brandon Snider
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brandon Snider 2006-07-25 22:10:07 UTC
Version:            (using KDE KDE 3.5.3)
Installed from:    Ubuntu Packages
OS:                Linux

Konq & Safari are failing to apply padding-bottom instructions on inline containers such as div with the display:inline attribute or span. The other 3 paddings work, but not padding-bottom.
Comment 1 Maksim Orlovich 2006-07-25 22:22:05 UTC
Any chance you could attach a testcase? Thanks.
Comment 2 Brandon Snider 2006-07-25 22:48:30 UTC
<html>
This should appear as a link surrounded by red colour. In the code, this container is "navbar". On the left and right, there is padding applied successfully. There is none applied to the top and bottom. But it is in the code. Now look at the page in Firefox or Opera. You can clearly see the padding applied successfully.
<div><FONT face="Lucida Handwriting, Cursive"><EM>- Brandon J. Snider</EM></FONT></div></html>




[bugs.kde.org quoted mail]

_________________________________________________________________
Play Q6 for your chance to WIN great prizes.  http://q6trivia.imagine-live.com/enca/landing


Created an attachment (id=17127)
testcase.html
Comment 3 Allan Sandfeld 2006-07-31 12:03:59 UTC
Inline elements doesn't have padding. If I remember correctly MSIE has some horrible quirk where they change display:inline to display:block if the inline elements gets padding or border.
Comment 4 Allan Sandfeld 2006-07-31 12:09:26 UTC
On the other hand Firefox and Opera does it too. 

The really silly thing here is that the padding of the DIV and that of SPAN is "collapsing". This is really quirky.
Comment 5 Brandon Snider 2006-07-31 21:08:38 UTC
<html>
Why should there be padding on the outsides but not on the top and bottom of inline elements?
<div><FONT face="Lucida Handwriting, Cursive"><EM>- Brandon J. Snider</EM></FONT></div></html>




[bugs.kde.org quoted mail]

_________________________________________________________________
Play Q6 for your chance to WIN great prizes.  http://q6trivia.imagine-live.com/enca/landing
Comment 6 Germain Garand 2006-07-31 23:30:32 UTC
re: #3/#4

mmh, after investigation the bug report looks valid to me. There's no provision in CSS against padding being applied to inline flow elements, so it definetly should be applied to the line boxes.

> The really silly thing here is that the padding of the DIV and that of SPAN 
> is "collapsing".

It isn't silly if you consider it doesn't affect the line boxes' line-height, so doesn't add up to the line spacing.

This is clearly stated in 10.8.1 (emphasis is mine):

"Although margins, borders, *and padding* of non-replaced elements do not enter into the line box calculation, *they are still rendered around inline boxes*. This means that if the height specified by 'line-height' is less than the content height of contained boxes, backgrounds and colors of padding and borders may "bleed" into adjoining line boxes. User agents should render the boxes in document order. This will cause the borders on subsequent lines to paint over the borders and text of previous lines. "

Since it has no spacing effect it went mostly unnoticed, but it does look like an inline model screw up in khtml.
Comment 7 Allan Sandfeld 2006-08-01 09:42:26 UTC
I came to the same conclusion after investigating. Note that border-top and border-bottom share the same problem. While the borders are painted their width does not contribute to the height, and thus thick borders are sometimes painted on top of the text.
Comment 8 Germain Garand 2006-08-23 04:39:50 UTC
turns out it's a problem of depth of in line box's hasTextChildren flag.

hasTextChildren only says if a linebox has immediate text children (e.g in testcase, removing the intermediate <a> tag will solve problem).

It's important to have it in order to know if a root "strut" should be applied to current line or not, but it has also been used abusively for things like shrinking of line boxes with no text children in quirk mode.

Those should definetly have an hasTextDescendant flag.
Comment 9 Germain Garand 2006-08-23 04:50:21 UTC
SVN commit 576084 by ggarand:

introduce hasTextDescendant flag for inline boxes.
hasTextChildren is good for evaluating a root's strut immediate relevance, 
but not for top/bottom overflow and other quirky shrinking. Those need depth.

BUG: 131366



 M  +3 -3      render_line.cpp  
 M  +9 -2      render_line.h  


--- branches/KDE/3.5/kdelibs/khtml/rendering/render_line.cpp #576083:576084
@@ -441,7 +441,7 @@
         else if (curr->yPos() == PositionBottom)
             curr->setYPos(y + maxHeight - curr->height());
         else {
-            if (!curr->hasTextChildren() && !strictMode)
+            if (!strictMode && !curr->hasTextDescendant())
                 childAffectsTopBottomPos = false;
             curr->setYPos(curr->yPos() + y + maxAscent - curr->baseline());
         }
@@ -499,7 +499,7 @@
             setBaseline(ascent);
         }
 #endif
-        if (hasTextChildren() || strictMode) {
+        if (hasTextDescendant() || strictMode) {
             if (yPos() < topPosition)
                 topPosition = yPos();
             if (yPos() + height() > bottomPosition)
@@ -520,7 +520,7 @@
     }
 
     // See if we have text children. If not, then we need to shrink ourselves to fit on the line.
-    if (!hasTextChildren()) {
+    if (!hasTextDescendant()) {
         if (yPos() < topPos)
             setYPos(topPos);
         if (yPos() + height() > bottomPos)
--- branches/KDE/3.5/kdelibs/khtml/rendering/render_line.h #576083:576084
@@ -96,6 +96,7 @@
     int baseline() const { return m_baseline; }
 
     virtual bool hasTextChildren() const { return true; }
+    virtual bool hasTextDescendant() const { return true; }
 
     virtual int topOverflow() const { return yPos(); }
     virtual int bottomOverflow() const { return yPos()+height(); }
@@ -155,6 +156,7 @@
         m_lastChild = 0;
         m_includeLeftEdge = m_includeRightEdge = false;
         m_hasTextChildren = false;
+        m_hasTextDescendant = false;
         m_afterPageBreak = false;
     }
 
@@ -180,8 +182,11 @@
         }
         child->setFirstLineStyleBit(m_firstLine);
         child->setParent(this);
-        if (child->isInlineTextBox())
-            m_hasTextChildren = true;
+        if (!m_hasTextChildren && child->isInlineTextBox()) {
+            m_hasTextDescendant = m_hasTextChildren = true;
+            for (InlineFlowBox* p = m_parent; p && !p->hasTextDescendant(); p = p->parent())
+                p->m_hasTextDescendant = true;
+        }
     }
     void removeFromLine(InlineBox* child);
     virtual void paintBackgroundAndBorder(RenderObject::PaintInfo&, int _tx, int _ty, int xOffsetOnLine);
@@ -207,6 +212,7 @@
         m_includeRightEdge = includeRight;
     }
     virtual bool hasTextChildren() const { return m_hasTextChildren; }
+    bool hasTextDescendant() const { return m_hasTextDescendant; }
 
     // Helper functions used during line construction and placement.
     void determineSpacingForFlowBoxes(bool lastLine, RenderObject* endObject);
@@ -235,6 +241,7 @@
     bool m_includeLeftEdge : 1;
     bool m_includeRightEdge : 1;
     bool m_hasTextChildren : 1;
+    bool m_hasTextDescendant : 1;
     bool m_afterPageBreak : 1;
 };