Closed Bug 930218 Opened 6 years ago Closed 4 years ago
Parent shrink-wrap width too wide when child has intrinsic ratio and percent height and ancestor has border-box box-sizing
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release) Build ID: 20130910160258 Steps to reproduce: Put a canvas(max-height:100%) inside a floating div(with fixed height and box-sizing:border-box) Actual results: A gap appears between right sides of the elements (2 x wrapper's border-size) Expected results: Wrapper should wrap the canvas without any gap. Wrapper does not have to be a floating div. An inline-block element would also behave similarly.
Status: UNCONFIRMED → NEW
Component: General → Layout
Ever confirmed: true
Version: unspecified → Trunk
Seems likely to be something wrong with the code in nsLayoutUtils::IntrinsicForContainer that handles elements with an intrinsic ratio -- probably either GetPercentHeight or some of its callers in IntrinsicForContainer need to handle box-sizing. (All this code is in layout/base/nsLayoutUtils.cpp.) At first glance, it seems likely that the problem is that GetPercentHeight needs to consider the box-sizing value on the containing block, and use the containing block's content-box height. Then again, I'm assuming that percentage heights should always be relative to the content-box height, no matter what box-sizing is. I think that's probably correct, but also likely unspecified. (Note, however, that GetPercentHeight should *not* consider the box-sizing on aFrame; the callers appear to do that, though it's worth checking that they all do it correctly.) So fixing this bug requires: (a) understanding the GetPercentHeight callers and checking whether the above is correct (b) fixing as appropriate (c) writing a reftest Looking at the code, I'm also very suspicious of the code in IntrinsicForContainer that uses GetPercentHeight on padding -- that seems wrong, since percentage padding is relative to widths.
David, does this need to block unprefixing box-sizing?
I don't have a strong opinion either way, but I'm inclined to think it does not.
Whiteboard: [mentor=dbaron] → [mentor=dbaron][lang=c++]
Dear David, I'm a new comer to the Mozilla code base. Could I work on this bug as a first bug ?
This is probably a bit too much for a *first* bug; you're better off starting with something else, I think.
Thank you for the advice, I'll find something easier to start with. Good luck squashing this one, François
Hi, I am interested in working with the bug. So please can you assign it to me? Regards, Anup
Sure. One other note about this bug: it's likely to involve fixes for a few separate problems. It's best to keep fixes for separate problems in separate patches so that they're easier to review
Assignee: nobody → allamsetty.anup
Whiteboard: [mentor=dbaron][lang=c++] → [lang=c++]
dbaron, I tried the nightly version and still can reproduce. I am interested in this bug. May I take this bug?
Yes, absolutely. (Sorry for the delay responding; was on vacation.)
Summary: Canvas not wrapped by parent that has border-box sizing → Parent shrink-wrap width too wide when child has intrinsic ratio and percent height and ancestor has border-box box-sizing
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8696088 [details] [diff] [review] part 1. Factor out the computation of block-size taken up by box-sizing into a separate function so we can reuse it >+ // XXXbz Calling GetPercentBSize on padding values looks bogus, since >+ // percent padding is always a percentage of the inline-size of the >+ // containing block. We should perhaps just treat non-absolute paddings >+ // here as 0 instead. Maybe file a bug on this?
Attachment #8696088 - Flags: review?(dbaron) → review+
Attachment #8696089 - Flags: review?(dbaron) → review+
> Maybe file a bug on this? Bug 1231059 filed.
You need to log in before you can comment on or make changes to this bug.