Last Comment Bug 571281 - inside list marker not drawn if list-item has overflow other than visible
: inside list marker not drawn if list-item has overflow other than visible
Status: RESOLVED FIXED
: css2, regression
Product: Core
Classification: Components
Component: Layout: Block and Inline (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.3a5
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on: 571618
Blocks: 456484
  Show dependency treegraph
 
Reported: 2010-06-10 10:54 PDT by Daniel.S
Modified: 2010-06-11 20:46 PDT (History)
2 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
simple testcase (359 bytes, text/html)
2010-06-10 10:54 PDT, Daniel.S
no flags Details
Fix the regression (3.42 KB, patch)
2010-06-10 13:53 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Review
Like so? (4.95 KB, patch)
2010-06-10 20:23 PDT, Boris Zbarsky [:bz]
roc: review+
Details | Diff | Review

Description Daniel.S 2010-06-10 10:54:53 PDT
Created attachment 450389 [details]
simple testcase

If I set overflow of a list-item to anything except 'visible', the marker box is never rendered.

Note that this bug is about overflow applied to list items, not on list item containers like ul or ol where we have enough bugs about.

In the case of list-style-position: outside, the marker has apparently never been rendered in Gecko history.
In the case of list-style-position: inside, there is a regression.

I don't have a smaller range for that than Firefox 3.0 worked, 3.6 up to current nightlies don't work.

Currently CSS 2.1 defines

> 'overflow' on the element does not clip the marker box.

See http://www.w3.org/TR/CSS2/generate.html#propdef-list-style-position
Comment 1 Boris Zbarsky [:bz] 2010-06-10 12:55:37 PDT
We're not even creating the bullet frame in those cases (which is why the "inside" case is broken too).
Comment 2 Boris Zbarsky [:bz] 2010-06-10 13:07:24 PDT
OK, that regressed when we stopped inheriting the display:list-item into the scrolledframe in bug 456484.
Comment 3 Boris Zbarsky [:bz] 2010-06-10 13:53:35 PDT
Created attachment 450441 [details] [diff] [review]
Fix the regression

I'm really not sure how to make the scrollframe not clip the bullet (or how that would even behave... when scrolling presumably the bullet would need to be _outside_ the scrollframe?).  We should spin that off into a separate bug.
Comment 4 Boris Zbarsky [:bz] 2010-06-10 13:56:16 PDT
Though I should note that neither webkit nor presto implement the outside bullet behavior, so either it's a recent spec change or it's seriously at risk.
Comment 5 Daniel.S 2010-06-10 14:08:51 PDT
(In reply to comment #4)
> Though I should note that neither webkit nor presto implement the outside
> bullet behavior, so either it's a recent spec change or it's seriously at risk.

The definition was added between the 19 July 2007 and 23 April 2009 CRs.

IE doesn't implement that behaviour either (I was about to file a bug over there).

However, if I got a list with a lot of content in each list-item, and I clip that using overflow, I'd still expect the bullets to render. There seems to be no other way that doesn't require additional code.
Comment 6 Boris Zbarsky [:bz] 2010-06-10 14:12:46 PDT
Right; I understand why the spec's proposed behavior is desirable.  It just happens to be a huge pain to implement given the other constraints the spec places on list bullets (like inheriting various properties from the list, for example!).
Comment 7 Daniel.S 2010-06-10 14:23:09 PDT
Yeah, I thought so..

I still think the spec shouldn't change. Else we probably cut off a path we'd like to go in the future. But for now the implementation would be low priority.

Hmm, thinking of Level 3 Lists out of interest. Can we estimate whether the implementation of a ::marker pseudo-element (or whatever it will end up with) would ease the difficulty of this problem?
Comment 8 Boris Zbarsky [:bz] 2010-06-10 14:25:43 PDT
Not really.  We effectively have an internal pseudo-element for the bullet already.
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-10 18:26:00 PDT
Comment on attachment 450441 [details] [diff] [review]
Fix the regression

Hmm, I wonder if this will fail if you have -moz-column set on the list-item? Maybe we should check the primary frame of the frame's element?
Comment 10 Boris Zbarsky [:bz] 2010-06-10 19:02:11 PDT
I considered that, but was worried about table pseudo stuff.  I guess that shouldn't be an issue, though....  OK, I'll do that.  And add a test for the column case (though how columns are supposed to interact with bullets... <sigh>).
Comment 11 Boris Zbarsky [:bz] 2010-06-10 20:05:23 PDT
> Hmm, I wonder if this will fail if you have -moz-column set on the list-item?

Yes, indeed.  I'll add a test for that.
Comment 12 Boris Zbarsky [:bz] 2010-06-10 20:12:47 PDT
But we can't use the primary frame, because in SetInitialChildList (which is where this code is) that's not set yet.
Comment 13 Boris Zbarsky [:bz] 2010-06-10 20:23:08 PDT
Created attachment 450570 [details] [diff] [review]
Like so?
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-11 08:56:27 PDT
Comment on attachment 450570 [details] [diff] [review]
Like so?

This works, but only because an outer table frame can't appear here (since table frames can't be NS_DISPLAY_LIST_ITEM). You might want to mention that in a comment.
Comment 15 Boris Zbarsky [:bz] 2010-06-11 12:13:43 PDT
> You might want to mention that in a comment.

Done.
Comment 16 Boris Zbarsky [:bz] 2010-06-11 12:27:46 PDT
Pushed http://hg.mozilla.org/mozilla-central/rev/574993ec2aee

Filed bug 571564 on outside markers.

Note You need to log in before you can comment on or make changes to this bug.