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

RESOLVED FIXED in mozilla1.8alpha1

Status

()

Core
Layout: Block and Inline
P1
normal
RESOLVED FIXED
15 years ago
12 years ago

People

(Reporter: Paul Arzul, Assigned: bz)

Tracking

({css2, testcase})

Trunk
mozilla1.8alpha1
css2, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

15 years ago
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

15 years ago
This may be related to bug 187419.
Confirm new 20030516 PC/WinXP
OS->all
->Layout B&I
Assignee: dbaron → block-and-inline
Status: UNCONFIRMED → NEW
Component: Style System → Layout: Block & Inline
Ever confirmed: true
OS: Linux → All
Created attachment 126151 [details]
minimization of that testcase
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

14 years ago
Right on, Boris. I found that adding &nbsp; (non-breaking space) right before
the block element is a workaround.

Updated

14 years ago
Keywords: css2, testcase
Summary: [css2] list-style-images load only when resizing browser → list-style-images load only when resizing browser
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?

Updated

14 years ago
Hardware: PC → All
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?
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.
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.
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.
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.)
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.
Created attachment 145868 [details] [diff] [review]
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
Assignee: core.layout.block-and-inline → bzbarsky
Status: NEW → ASSIGNED
Attachment #145868 - Flags: superreview?(dbaron)
Attachment #145868 - Flags: review?(dbaron)
Priority: -- → P1
Summary: list-style-images load only when resizing browser → [FIX]list-style-images load only when resizing browser
Target Milestone: --- → mozilla1.7final
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);
>+    }
>>+        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.
Attachment #145868 - Flags: superreview?(dbaron)
Attachment #145868 - Flags: review?(dbaron)
Created attachment 145945 [details] [diff] [review]
Updated to comments
Attachment #145868 - Attachment is obsolete: true
Attachment #145945 - Flags: superreview?(dbaron)
Attachment #145945 - Flags: review?(dbaron)
Did you want to include the nsBulletFrame.cpp diffs?
Attachment #145945 - Flags: superreview?(dbaron)
Attachment #145945 - Flags: review?(dbaron)
Created attachment 145949 [details] [diff] [review]
Doh

Yes, I did.... those are the same as before.  Sorry about that.
Attachment #145945 - Attachment is obsolete: true
Attachment #145949 - Flags: superreview?(dbaron)
Attachment #145949 - Flags: review?(dbaron)
Attachment #145949 - Flags: superreview?(dbaron)
Attachment #145949 - Flags: superreview+
Attachment #145949 - Flags: review?(dbaron)
Attachment #145949 - Flags: review+
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.
Summary: [FIX]list-style-images load only when resizing browser → [FIXr]list-style-images load only when resizing browser
Checked in for 1.8a; leaving open pending David deciding whether this is ok for 1.7.
I don't think it's worth trying to get this into the branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.7final → mozilla1.8alpha
You need to log in before you can comment on or make changes to this bug.