The text-overflow: ellipsis seems to behave incorrectly within an element of display:-moz-box; hopefully this is a real bug. I'm attaching a test html page to show the differences and will have compare screenshots with (chrome) webkit. Each item has the class "bacon" which gives the following properties: .bacon { white-space: nowrap; text-overflow: ellipsis; overflow: hidden; width: 400px; } The first set is variations of wrapping a block with a box element. 1: display:block; ellipsis happens correctly here in both webkit and gecko 2: display:-moz-box; ellipsis happens correctly in webkit (it likely assumes a block display as it ignores -moz prefixes). in gecko the block seems to ignore the width property which could be another unrelated bug. 3: display:-webkit-box; ellipsis happens correctly in webkit and in gecko (which likely assumes a block as it ignores -webkit prefixes) 4: display:box; ellipsis happens correctly as both webkit and gecko likely assume a block display The second set is using a box directly as the text content parent. 1: display:block; ellipsis happens correctly here in both webkit and gecko 2: display:-moz-box; ellipsis happens correctly in webkit (it likely assumes a block display as it ignores -moz prefixes). in gecko the text is truncated but without the ellipsis 3: display:-webkit-box; ellipsis happens correctly in gecko (which likely assumes a block as it ignores -webkit prefixes) in webkit the text is truncated but without the ellipsis 4: display:box; ellipsis happens correctly as both webkit and gecko likely assume a block display

This is a screenshot of google chrome canary, the rendering is the same in google chrome stable. Just to note that the Firefox test was done with nightly version: 8.0a1 (2011-07-20)

I think this has rather little to do with text-overflow:ellipsis and a lot to do with the sizes that the box code makes things.

Ha! I just ran into this while trying to use text-overflow: ellipsis. The second line is the one that I'd like to see triggering text-overflow (obviously). I also tried constraining the inner .bacon with top: 0 left: 0 right: 0 bottom: 0 so that it would be exactly as large as the size allocated to its parent box, but that didn't seem to help. My testcase: http://jonathan.protzenko.free.fr/css/mozbox.html

How can I help to get this fixed? (besides coding, which I'm not into)

Mats, could you possibly give a rough outline of what to do in order to fix this? I have some time on my hands during the holiday season, and if this is within reach, I might as well try to fix it. (I suppose you know how to do this since you implemented text-overflow: ellipsis in the first place.) Thanks :)

So after looking into the problem, here's what I've been able to gather so far. - The problem happens when we ask the outer block frame "what's your min width?". The block frame goes on asking its lines to advertise how much min width they want: this is (if I'm not mistaken) nsBlockFrame.cpp:734 kid->AddInlineMinWidth(aRenderingContext, &data); - The underlying text frame then complies with the request, and the min width is bumped in nsTextFrameThebes.cpp:6536 aData->currentLine = NSCoordSaturatingAdd(aData->currentLine, width); Skipping that line "solves" the problem. - My intuition is that we should be smarter in nsBlockFrame::GetMinWidth. That is, if the block frame has text-overflow: clip or ellipsis, this means that its min width is zero, because we can hide as much text as needed. Thus, we should not even proceed with the call to AddInlineMinWidth if we're in a situation of text-overflow. - I don't want to duplicate the whole logic that decides whether TextOverflow should be effective. However, it seems I can't reuse the logic from TextOverflow::CanHaveTextOverflow, because that function is written to be used from DisplayBuildList, not in the context of GetMinWidth. I'll see if I can hack something up :).

Hi Mats, I'd be happy to hear your thoughts on this. Feel free to redirect the review if you're not the right person! The patch isn't worth much, but I could use some guidance on how to do this properly. Thanks, jonathan

Comment on attachment 585221 [details] [diff] [review] Proof-of-concept + mochitest I don't think we should mess with min/pref sizes or anything that affects layout.

I think the root of the problem is that the nsXULScrollFrame child frame doesn't have a sane scrollable overflow area. One approach that might work is to treat the -moz-xul-anonymous-block pseudo frame specially by using its ancestor scroll frame when calculating the clip area. As you can see it doesn't have any scrollable oveflow, nor does its lines, so you need to force your way past those checks in the TextOverflow code.

... and by "calculating the clip area" I mean TextOverflow::mContentArea here: http://mxr.mozilla.org/mozilla-central/source/layout/generic/TextOverflow.cpp#262 that's the area that's used for ellipsing.

roc, do you see a better solution?

(In reply to Mats Palmgren [:mats] from comment #11) > I think the root of the problem is that the nsXULScrollFrame child > frame doesn't have a sane scrollable overflow area. Seems like that should be fixed directly.

Sure, I'll try to do that. But what's the intuition with GetMinWidth? If it doesn't really return a "minimum width", what is the function's goal exactly?

Also, Mats, how did you get the frame dump of only the HTML contents? Whenever I use -layoutdebug and hit Dump > Frames, it dumps the frames for the entire window, including its chrome, and not just the html content area.

Start Firefox normally, then Tools->Layout Debugger.

(In reply to Jonathan Protzenko [:protz] from comment #15) > Sure, I'll try to do that. But what's the intuition with GetMinWidth? If it > doesn't really return a "minimum width", what is the function's goal exactly? It returns the intrinsic minimum width, which is the width required to lay out the element without overflowing its container if you assume that all optional line breaks are taken.

Huh I'm confused because the Box frame that's right under the XULScroll frame does have the "right" scrollable area. (I'm assuming that's what you referred to when you wrote "the nsXULScrollFrame child frame doesn't have a sane scrollable overflow area".) (gdb) x/wa *({void**}0x7ffff6d04820) 0x7ffff263b78c <nsXULScrollFrame::QueryFrame(nsQueryFrame::FrameIID)>: 0xffffffffe5894855 (gdb) p ({nsXULScrollFrame}0x7ffff6d04820)->mInner.mScrolledFrame->GetScrollableOverflowRect() [Thread 0x7fffd05c9700 (LWP 20217) exited] $42 = {<mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = { x = 0, y = 0, width = 29460, height = 1140}, <No data fields>} However, its content area is indeed too big: (gdb) p ({nsXULScrollFrame}0x7ffff6d04820)->mInner.mScrolledFrame->mRect $43 = {<mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = { x = 0, y = 0, width = 29460, height = 1140}, <No data fields>} That's as far as I could get, I'll try to figure out why the nsBoxFrame insists on having 24960 units as its width while the stylesheet explicitly requests 24000. It may be the nsXULScrollFrame allocates too much width for its child, or the nsBoxFrame ignores the stylesheet (I guess...).

GetScrollableOverflowRect() just returns the frame rect when there's no registered overflow areas, see the last line in nsIFrame::GetOverflowRect. (and in that case the "scr-overflow=" is omitted in the frame dump) (gdb) p this->mInner.mScrolledFrame->HasOverflowAreas() $1 = false FYI, you can use aFrame->List(stdout,0) in gdb to get a frame dump. > why the nsBoxFrame insists on having 24960 units as its width while the stylesheet > explicitly requests 24000 That's 16px, almost certainly the width of a scrollbar. Hmm, did you get that with the overflow:hidden testcase? Anyway, that's not important for this bug I think.

> layout/generic/nsGfxScrollFrame.cpp Make nsXULScrollFrame report overflow for the relevant frame. > layout/generic/TextOverflow.* Move initialisation code to a separate method, TextOverflow::Init. Add code for mozXULAnonymousBlock that get the scroll frame from the parent. Do the 1px tweak in this case also, to counter rounding of scroll positions by nsXULScrollFrame.

Jonathan, can you try the attached patch and see if it works for the feature you're working on? Thanks.

Comment on attachment 591212 [details] [diff] [review] reftest Review of attachment 591212 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/reftests/text-overflow/reftest.list @@ +20,5 @@ > HTTP(..) == table-cell.html table-cell-ref.html > HTTP(..) == two-value-syntax.html two-value-syntax-ref.html > HTTP(..) == single-value.html single-value-ref.html > HTTP(..) == atomic-under-marker.html atomic-under-marker-ref.html > +fuzzy(1,250000) skip-if(Android) HTTP(..) == xulscroll.html xulscroll-ref.html Er, why so many fuzzy pixels?

So your patch works just fine for me, unfortunately, it isn't enough for me to achieve what I want. I've put online another testcase at http://jonathan.protzenko.free.fr/bug672944.html : I wish the text-overflow: ellipsis would kick in for the second lime box, instead of making the outer box (.wrapper) larger than 400px. However, reading http://www.w3.org/TR/css3-flexbox/, I'm having a hard time figuring out what is the "correct" behavior. I'm not even sure what I want is possible with the current flex box model...

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #24) >> +fuzzy(1,250000) skip-if(Android) HTTP(..) == xulscroll.html xulscroll-ref.html > Er, why so many fuzzy pixels? As long as they only differ by 1 in color it's due to anti-aliasing and I don't really care how many AA pixels there are.

(In reply to Jonathan Protzenko [:protz] from comment #25) > So your patch works just fine for me, unfortunately, it isn't enough for me > to achieve what I want. I've put online another testcase at > http://jonathan.protzenko.free.fr/bug672944.html : I wish the text-overflow: > ellipsis would kick in for the second lime box, instead of making the outer > box (.wrapper) larger than 400px. Please note that 'text-overflow' does not affect layout in any way (that is, sizes and positions of boxes), it's applied after layout. Maybe what you want is: .wrapper > * { -moz-box-flex: 1; display: -moz-box; } > However, reading > http://www.w3.org/TR/css3-flexbox/, I'm having a hard time figuring out what > is the "correct" behavior. I'm not even sure what I want is possible with > the current flex box model... -moz-box pre-dates the css3-flexbox spec by many years so you shouldn't expect that it matches what the spec says (they are quite unrelated). We are working on implementing css3-flexbox in bug 666041.

Oh thanks, right, I'd forgotten, this is exactly what I needed and the bug precisely fixes this. Sorry for the noise, fixing this bug indeed solves my very issue :).

(In reply to Mats Palmgren [:mats] from comment #26) > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #24) > >> +fuzzy(1,250000) skip-if(Android) HTTP(..) == xulscroll.html xulscroll-ref.html > > Er, why so many fuzzy pixels? > As long as they only differ by 1 in color it's due to anti-aliasing and I > don't really care how many AA pixels there are. Can you put the correct number in there anyway?

https://hg.mozilla.org/integration/mozilla-inbound/rev/bdbd188e6f5f https://hg.mozilla.org/integration/mozilla-inbound/rev/8f3fbc4a9c2a

https://hg.mozilla.org/mozilla-central/rev/8f3fbc4a9c2a https://hg.mozilla.org/mozilla-central/rev/bdbd188e6f5f