Closed
Bug 206602
Opened 22 years ago
Closed 21 years ago
[FIXr]list-style-images load only when resizing browser
Categories
(Core :: Layout: Block and Inline, defect, P1)
Core
Layout: Block and Inline
Tracking
()
RESOLVED
FIXED
mozilla1.8alpha1
People
(Reporter: habere, Assigned: bzbarsky)
References
()
Details
(Keywords: css2, testcase)
Attachments
(2 files, 2 obsolete files)
577 bytes,
text/html
|
Details | |
3.38 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
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•22 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
Assignee | ||
Comment 2•21 years ago
|
||
Assignee | ||
Comment 3•21 years ago
|
||
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•21 years ago
|
||
Right on, Boris. I found that adding (non-breaking space) right before
the block element is a workaround.
Updated•21 years ago
|
Assignee | ||
Comment 5•21 years ago
|
||
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•21 years ago
|
Hardware: PC → All
Comment 6•21 years ago
|
||
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?
Assignee | ||
Comment 7•21 years ago
|
||
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•21 years ago
|
||
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.
Assignee | ||
Comment 9•21 years ago
|
||
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•21 years ago
|
||
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.)
Assignee | ||
Comment 11•21 years ago
|
||
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.
Assignee | ||
Comment 12•21 years ago
|
||
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
Assignee | ||
Updated•21 years ago
|
Attachment #145868 -
Flags: superreview?(dbaron)
Attachment #145868 -
Flags: review?(dbaron)
Assignee | ||
Updated•21 years ago
|
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);
>+ }
Assignee | ||
Comment 14•21 years ago
|
||
>>+ 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.
Assignee | ||
Updated•21 years ago
|
Attachment #145868 -
Flags: superreview?(dbaron)
Attachment #145868 -
Flags: review?(dbaron)
Assignee | ||
Comment 15•21 years ago
|
||
Attachment #145868 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #145945 -
Flags: superreview?(dbaron)
Attachment #145945 -
Flags: review?(dbaron)
Did you want to include the nsBulletFrame.cpp diffs?
Assignee | ||
Updated•21 years ago
|
Attachment #145945 -
Flags: superreview?(dbaron)
Attachment #145945 -
Flags: review?(dbaron)
Assignee | ||
Comment 17•21 years ago
|
||
Yes, I did.... those are the same as before. Sorry about that.
Attachment #145945 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
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+
Assignee | ||
Comment 18•21 years ago
|
||
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.
Assignee | ||
Updated•21 years ago
|
Summary: [FIX]list-style-images load only when resizing browser → [FIXr]list-style-images load only when resizing browser
Assignee | ||
Comment 19•21 years ago
|
||
Checked in for 1.8a; leaving open pending David deciding whether this is ok for 1.7.
Assignee | ||
Comment 20•21 years ago
|
||
I don't think it's worth trying to get this into the branch.
Status: ASSIGNED → RESOLVED
Closed: 21 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.
Description
•