206602 - [FIXr]list-style-images load only when resizing browser

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect, P1)

Description

[habere-et-dispertire]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030430 Debian/1.3-5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030430 Debian/1.3-5

in the test url there are a number of custom bullets set with css2
list-style-images. for example:

li.aries
{
  list-style: url(aries24.png);
}

none of these are loaded. the bullets are either entirely missing or appear as
some blotched dot graphic.


Reproducible: Always

Steps to Reproduce:
to reproduce:

1. visit: http://home.worldonline.co.za/~astrology/faq/#6

(when the page has finished loading) look at the list items:
aries, taurus, etc.

now **resize** your browser, and watch the list-style-images suddenly appear.

it's as if the page isn't being redrawn for the final rendering.




this has happened with every version of mozilla i've ever tried
(1.0, 1.1, 1.2, 1.3 and 1.4b)

Comment 1

[Bill Mason]

This may be related to bug 187419.
Confirm new 20030516 PC/WinXP
OS->all
->Layout B&I

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: minimization of that testcase

Comment 3

[Boris Zbarsky [:bzbarsky]]

Shift-reloading the minimal testcase shows the problem....

It looks like this is a bug with an <li> whose first child is a block.  Somewhow
we're not reflowing/repainting the bullet frame correctly....

Comment 4

[Garrett Smith]

Right on, Boris. I found that adding   (non-breaking space) right before
the block element is a workaround.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So the problem here is that the initial reflow gives the bullet frame a size of
0x0 (not quite sure why, but it really doesn't matter -- even if it gave it the
size of a bullet in the current font the rest of the problem would exist).

Now when the bullet discovers its size it tries to reflow itself.  It does this
by calling ReflowDirtyChild() on the first child of the block of the block
frame.  that's pretty bogus...  What happens here is that the first child is a
height-0 textframe or something along those lines.  So we dirty the first line
in the block (height 0), and eventually end up in nsBlockFrame::PlaceLine, where
we skip reflowing the bullet, since the line height is 0 and it's not the last line.

In my opinion the right way to fix this is to have the bullet ReflowDirtyChild
itself and make blocks compare the child to mBullet and if the two are equal
determine which line to dirty itself (the whole "first line unless it's height
0, etc" mess that bullet frame knows about).  Even so, I'm not sure what would
happen if we have an inline first line of nonzero height and then a block and
the reflow makes the first line be 0.  Would the block end up getting reflown? 
If not, then we could still run into this issue of the bullet frame not getting
reflown when it needs to be.

David?  Thoughts?

Comment 6

[Hixie (not reading bugmail)]

Would making our bullet code actually work per spec (i.e. take part in the
inline box model for the first line of its block) help at all?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Hmm.. Should it do so even when the first line of the block is a zero-height
whitespace-only line?  That's what we have here.

Comment 8

[Hixie (not reading bugmail)]

What is a zero-height white-space only line? If white-space is normal, you just
don't get a line at all:

# Whitespace content that would subsequently be collapsed away according to the 
# 'white-space' property does not generate any anonymous inline boxes.
 -- http://www.w3.org/TR/CSS21/visuren.html#q7

Anyway, to answer your question, the current internal draft says that the marker
box will be vertically aligned with the first line of content in the principal
box, as specified by the pseudo-element's 'vertical-align' property. (The
principal box is the main one generated for the element with its 'display'
property set to list-item, as opposed to the marker box.) The marker box
participates in the height calculations of the principal box's first line box.
Thus, markers are aligned with the first line of an element's content. The first
line of a principal box is the one matched by the box's '::first-line'
pseudo-element. It might not be a direct child of the box, but it must be in
flow. If no first line box exists in a principal box, the marker box establishes
its line box alone, as the first box of the principal block box.

HTH.

Comment 9

[Boris Zbarsky [:bzbarsky]]

The issue is that we have this extra line floating around that doesn't actually
affect layout (as far as I can tell) but does exist in our internal data
structures that happens in cases like this:

<div style="display: list-item">

<div style="display: block">
</div>
</div

In this case, the inner div is actually in the second "line"; the first line is
an inline-level line containing no text.  This is sort of required if one
demands that changing the value of white-space does not lead to box creation
(which in Mozilla it does not; changing white-space just reflows, does not reframe).

The point is, most of the code involved knows about this situation and correctly
attaches the bullet to the right line.  The code I pointed out in comment 5 is
the exception.

Now there may be better ways to architect the whole thing.  I don't know.  I
don't understand block reflow quite well enough yet.

Comment 10

[Hixie (not reading bugmail)]

Ah. Well, those lines don't exist per CSS, so no, it wouldn't put it in those
lines. (Changing white-space should _so_ just reframe, if that would get around
having these random non-lines complicating layout everywhere... no? Seems like
changing white-space on the fly is pretty rare outside my test cases.)

Comment 11

[Boris Zbarsky [:bzbarsky]]

No idea, frankly.  Like I said, I really don't know block/line layout that well.
 Reframing is really expensive, though.  And dynamic changes will happen more if
I get async stylesheet loading working.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

This is the right fix, I think.  At the very least, it encapsulates all the
"where does the bullet live?" logic in nsBlockFrame, so it can be changed as a
unit as needed.  This patch fixes the testcase in this bug and the one in bug
210132

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 145868 [details] [diff] [review]
Fix

>+    if (aChild == mBullet && HaveOutsideBullet()) {
>+      // The bullet lives in the first line, unless the first line has
>+      // height 0, in which case it lives in the second line.
>+      line_iterator bulletLine = begin_lines();
>+      if (bulletLine != end_lines() && bulletLine->mBounds.height == 0) {
>+        MarkLineDirty(bulletLine);

why?

>+        bulletLine = bulletLine.next();
>+      }
>+      
>+      if (bulletLine == end_lines()) {
>+        // no meaningful lines; reflow all of this block
>+        NS_ASSERTION(mParent, "Should really have a parent!");
>+        if (mParent) {
>+          mState |= NS_FRAME_IS_DIRTY;
>+          mParent->ReflowDirtyChild(aPresShell, this);
>+        }        

Is there a better way?	(Perhaps there isn't, but I really dislike
ReflowDirtyChild.)

>+      } else {
>+        MarkLineDirty(bulletLine);
>+      }        
>+    } else {
>+      // Mark the line containing the child frame dirty.
>+      line_iterator fline = FindLineFor(aChild);
>+      if (fline != end_lines())
>+        MarkLineDirty(fline);
>+    }

Comment 14

[Boris Zbarsky [:bzbarsky]]

>>+        MarkLineDirty(bulletLine);
>why?

Hmm... maybe that's not needed, yes.  I thought I needed it to handle the case
when a reflow would make that line a different height, but that shouldn't happen
if all that's dirty is the bullet (and otherwise, the line is already dirty). 
I'll remove that line.

> Is there a better way?  (Perhaps there isn't, but I really dislike
> ReflowDirtyChild.)

Hmmm... Looking through ReflowDirtyLines, that handles the empty-line-list case
already (it just reflows the bullet then), so all this code is in fact
unnecessary.  I'll remove it.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Did you want to include the nsBulletFrame.cpp diffs?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: Doh

Yes, I did.... those are the same as before.  Sorry about that.

Comment 18

[Boris Zbarsky [:bzbarsky]]

David, do you think this is safe enough for 1.7?  I suspect it is, but I'm not
quite as comfortable with blocks as I'd like to be to be sure here.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Checked in for 1.8a; leaving open pending David deciding whether this is ok for 1.7.

Comment 20

[Boris Zbarsky [:bzbarsky]]

I don't think it's worth trying to get this into the branch.