Closed Bug 504622 Opened 15 years ago Closed 8 years ago

Fieldsets never shrink below their min-intrinsic width, even if min-width is explicitly set to 0

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: riksoft, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Keywords: DevAdvocacy, testcase, Whiteboard: [DevRel:P2])

Attachments

(4 files, 4 obsolete files)

testcase from comment 0
423 bytes, text/html
Details
workaround.htm
491 bytes, text/html
Details
Part 1 updated to review comments
15.85 KB, patch
Details | Diff | Splinter Review
Part 2 updated to review comments
12.55 KB, patch
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.11) Gecko/2009060215 Firefox/3.0.11 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.11) Gecko/2009060215 Firefox/3.0.11 This example show the problem. IE and Opera, correctly show a fieldset containing a div with horizontal scrollbar. Firefox and Chrome (even worse) instead dont show the scroll. Removing <fieldset> make all working ok also in Firefox. Another way to have the scrollbar is writing the width of the div first div in pixel... but I want 100% because I need auto-resizing. <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "xhtml11.dtd"> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252" /> <title>fieldset</title> </head> <body> <fieldset> <div style="width:100%;height:100px; overflow:scroll;"> <div style="width:2000px;height:10px;background-color:red">test test test test</div> </div> </fieldset> </body> </html> Reproducible: Always Steps to Reproduce: 1.Try removing fieldset on the above example. 2. 3. Expected Results: The expected result is having horizontal scrollbar appear. The fieldset should act as other containers, e.g. a div. and don't have to change the way an inside div work. I don't found a workaround and I'm going to using styled div to resemble as a fieldset with legend.
Version: unspecified → 3.0 Branch
Status: UNCONFIRMED → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
Maybe inside the C code could be the same bug, but but 261037 is quite differente from this. bug 261037 reported that a fieldset can't have am overflow scroll. Instead I reported that a DIV inside a fieldset loses it's capability of overflow:scroll. So, while the 261037 could be even interpreted as a lack of feature of the fieldset, I report an interference of a fieldset over a DIV element. The fieldset modifify the behaviour of a div in the particular situation explained in my example.
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
Confirmed with Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:2.0b7pre) Gecko/20101005 Firefox/4.0b7pre
Status: UNCONFIRMED → NEW
Component: General → Layout: Form Controls
Ever confirmed: true
Keywords: testcase
Product: Firefox → Core
QA Contact: general → layout.form-controls
Version: 3.0 Branch → Trunk
Fieldsets in Gecko never shrink in width below their min intrinsic width, which is the min intrinsic width of their content area (ignoring legends). And the intrinsic min width of a scrollframe is the intrinsic min width of its content, for web compat. In particular, they do NOT do the same sizing as normal blocks, which this bug seems to expect; this is needed for web compat as well. Note that you'd get the same behavior if you put your scrollable thing inside an auto-width float, say. I agree that the result here is somewhat undesirable. Perhaps we should allow fieldsets to shrink below their min-intrinsic width... that needs some study about what web pages it would break.
Summary: Fieldset makes div overflow not working → Fieldsets never shrink below their min-intrinsic width
The problem is not fieldsets that never shrink: I don't bother fieldset dimension because fieldset should do NOTHING. The strange is that a div don't understand width:100% if inside a fieldset. Let's look at this: <fieldset style="width:100px;background-color:#00FF00"> <!-- <div style="width:100%;height:100px; overflow:scroll;background-color:#808080"> <div style="width:2000px;height:10px;background-color:red">test test</div> </div> </fieldset> --> fieldset behave right, adjusting its dimensione to 100px Now let's remove the rem. <fieldset style="width:100px;background-color:#00FF00"> <div style="width:100%;height:100px; overflow:scroll;background-color:#808080"> <div style="width:2000px;height:10px;background-color:red">test test</div> </div> </fieldset> Now the logic is completely reversed: childs dictate what parents must do and: 1) the deeper div give the dimension to the parent div 2) for strange reason overflow inside fieldset don't work and 1st div adapt itself to its child div dimension 3) fieldset adapts itself to its child dimension (the 1st div) Where is the logic? Let's suppose that is right that a fieldset adapt its dimension based on that of a child div: why its 1st child div don't behave as if it was outside a fieldset since that fieldset is submitted to it? And if fieldset is not submitted to it, then why div width:100% don't see the limits of the document/viewport as it happen when is outside the fieldset? So the question is: the fieldset is or not a container? Why adapt to its child? And if a div is "stronger" than fieldset, why this don't work? <div style="width:100%"> <fieldset style="width:100px;background-color:#00FF00"> <div style="width:100%;height:100px; overflow:scroll;background-color:#808080"> <div style="width:2000px;height:10px;background-color:red">test test</div> </div> </fieldset> <div> I don't see any consistency. On this point is obviuosly more consistent and logic, Opera and IE. Note: Now this example works also in Chrome. The only browser where don't work is Firefox <fieldset style="width:100px;background-color:#00FF00"> <div style="width:100%;height:100px; overflow:scroll;background-color:#808080"> <div style="width:2000px;height:10px;background-color:red">test test</div> </div> </fieldset>
> The strange is that a div don't understand width:100% if inside a fieldset. It does. It means 100% of the width of the fieldset, whatever that is. That's why the fieldset's width behavior matters. > Where is the logic? You did read comment 5, right? It completely describes the behavior. > and 1st div adapt itself to its child div dimension No, it just sets its width to the width of the fieldset, whatever that is. > the fieldset is or not a container? That's a meaningless question; there is no such concept in CSS terms. > Why adapt to its child? I mentioned this in comment 5, no? There are situations where fieldsets need to expand to the min-intrinsic width of their kids. Now perhaps these are not all situations; see the end of comment 5. > I don't see any consistency. It's really a very simple behavior. The width of the fieldset in this case is the maximum of its width as an auto-width block and the minimal intrinsic width of its descendants. See http://hg.mozilla.org/mozilla-central/file/dee1e01fd8ed/layout/forms/nsFieldSetFrame.cpp#l394 for the code if you care And yes, I agree that the behavior for fixed-width fieldsets is not desirable; it's there for compat with something in some cases, though. The question is what cases. Again, see end of comment 5.
(In reply to comment #7) > > The strange is that a div don't understand width:100% if inside a fieldset. > > It does. It means 100% of the width of the fieldset, whatever that is. That's > why the fieldset's width behavior matters. This seems a paradox; I say: div 100% of what? Of the fieldset. What is fieldset 100%? You say is the 100% of the div. But that's a nonsense loop especially with overflow:scroll I can understand that: fieldset with 100% or no width specified can expand to meet child div needs. BUT I don't understand why the inner div needs are to be 2000px when overflow:scroll! WHY? I can understand fieldset expansion if no overflow:scroll. But why overflow:scroll doesn't work if inside a fieldset? You say: because filedset adapt to div. Its like Einstein paradox of child and father in a time travel! :-) Worst yet tha using %, is the behaviuor with px If I say: fieldset 120px; and div 100%, I expect div to be 120px (as all the browser in the world do). So if then change 120px into 100% I expect fieldset and div to be the same width of the container of the fieldset (e.g. un upper div o the viewport). > > > Where is the logic? > > You did read comment 5, right? It completely describes the behavior. Yes but I find it unreal. Especially, but not limited, to this case: <div style="width:100px"> <fieldset style="width:100px;background-color:#00FF00"> <div style="width:100%;height:100px; overflow:scroll;background-color:#808080"> <div style="width:2000px;height:10px;background-color:red">test test</div> </div> </fieldset> <div> Why, - despite the px, - despite the first div container of 100px - despite the fieldset 100px - the everything enlarge to 2000px? =8-o I don't find a real, complete explanation in comment 5. It sounds like a cat that bites its tail.
> No, it just sets its width to the width of the fieldset, whatever that is. But nor fieldset to 100px, nor fieldset contained in a div if 100px, locks the fieldset to 100px or worst yet to 100% of the parent. So, where is the master, the lord of the width? :-) I find all quite illogical. > > the fieldset is or not a container? > > That's a meaningless question; there is no such concept in CSS terms. But it's a meaningful question for logical and consistent use (especially when 1 browser behave differetly against 1000 others that behave the same). > > > Why adapt to its child? > > I mentioned this in comment 5, no? There are situations where fieldsets need > to expand to the min-intrinsic width of their kids. Now perhaps these are not > all situations; see the end of comment 5. OK, I consider that on comment 6 and here again, but, consistency apart, don't explain why it expand in that situation where it hasn't any need to do that: why expand if is not needed?! We can reason: overflow:scroll dont' act because fieldset expands. why it expands? Because overflow:scroll don't work why don't work? Because fieldset expand! why it expands? Because overflow:scroll don't work why don't work? Because fieldset expand! why it expands? Because overflow:scroll don't work . . . PARADOX/loop > > > I don't see any consistency. > > It's really a very simple behavior. The width of the fieldset in this case is > the maximum of its width as an auto-width block and the minimal intrinsic width But why it expand if not required?! Because of overflow:scroll not working. Why not working?! Because of what? Fieldset expanded? Noooo because that should be a paradox. So because of what? My response is: because of a bug in the project: here we have that the inner div is 2000px; its parent div 100% should BEHAVE LIKE WITHOUT A Fieldset since fieldset is auto-inc and so should be ininfluent. So 100% should be 100% of the DOCUMENT, not of the universe. Instead we have a bud influence: fieldset make its child div to think the document width is infinite and overflow:scroll don't work. Not good! > of its descendants. See > http://hg.mozilla.org/mozilla-central/file/dee1e01fd8ed/layout/forms/nsFieldSetFrame.cpp#l394 > for the code if you care > > And yes, I agree that the behavior for fixed-width fieldsets is not desirable; > it's there for compat with something in some cases, though. The question is > what cases. Again, see end of comment 5. We should see how work GetMinWidth and what is result.width here nscoord minWidth = GetMinWidth(aRenderingContext); if (minWidth > result.width) anyway, code apart is a very strange behaviour and differ at least from Opera, Chrome and IE. There is no way to cope with this problem. I don't know any workaround other than avoid using fieldset. It's ok, not ok, correct, not correct... I don't know, but I'm sure that to me doesn't sound very good. :-) I don't test with 100% but at least with fixed-width Firefox 2.0 worked OK. The problem arose with 3.x Some browsers without this problem: Opera, IE, Chrome, Seamonkey 1 (not 2), Firefox 2, Iceape
> I don't understand why the inner div needs are to be 2000px when > overflow:scroll! Because it turns out that web pages depend on that behavior, in general. We tried making the min intrinsic width of scrollframes be 0 at one point, and all sorts of things on the web broke. > I don't find a real, complete explanation in comment 5. That seems to be your problem... comment 5 describes exactly the algorithm the browser is following here.
And it case it wasn't clear, the behavior here is very broken, I agree. The question is how to fix it without breaking other things. For example, changing the min-intrinsic width of scrollframes is a no-go.
> That seems to be your problem... comment 5 describes exactly the algorithm the browser is following here. Reading again comment 5 I probably found the tiny element of disturb: If I correctly understand, the fieldset expands based on the content of the scrollframe (the content of the div) and not based on the div width. This causes the div to see a parent of 2000px and expand also its width. Correct? >And it case it wasn't clear, the behavior here is very broken, I agree. The >question is how to fix it without breaking other things. For example, changing >the min-intrinsic width of scrollframes is a no-go. The compat has been already broken from 2.x to 3.x, so at least could be fixed the problem about fixed-width. About % width I don't know. On my part, the only workaround with Firefox is using JS or not using fieldset. The problem is to decide if it is worst to broken this compat, or keeping this strange behaviour by design that give another unwanted cross browsing problem to the developers. One solution could be the compatibility string into the page, as microzozz do in IE.
> the fieldset expands based on the content of the scrollframe Which in this case is a <div style="width: 2000px">, yes. > The compat has been already broken from 2.x to 3.x There were behavior changes from 2.x to 3.x; they by and large did not break sites, though. Compat means not breaking sites, not keeping the same behavior; else we'd never fix bugs. ;)
(In reply to comment #13) > There were behavior changes from 2.x to 3.x; they by and large did not break > sites, though. Compat means not breaking sites, not keeping the same behavior; > else we'd never fix bugs. ;) :-) I'm not too accustomed to philosophy or I'm dumb; in both case I don't understand the difference from "breaking sites" and "keeping the same behaviour". I only understand that if a site use: <div style="width:100px"> <fieldset style="width:100px;background-color:#00FF00"> <div style="width:100%;height:100px; overflow:scroll;background-color:#808080"> <div style="width:2000px;height:10px;background-color:red">test test</div> </div> </fieldset> <div> with 2.0 has a behaviour (100px), with 3.0 another one (2000px). This 2 different behaviour break very much a site because 100px is very different from 2000px and is surely devastating for any site. So it looks like a compat problem. However, do what you want. FF its not on me. I only hilight problems. Developers, as usual, will use some trickery, writing down another compatibility and cross browsing problem to be aware of.
You can add style="display:table-column;" to Fildset as a Workaround! regards, sufnoK
Blocks: 823483
See also: http://stackoverflow.com/questions/17408815/fieldset-resizes-wrong-appears-to-have-unremovable-min-width-min-content The current behavior seems somewhat in line with the spec (modulo an !important, note the `min-width: min-content;` on https://html.spec.whatwg.org/multipage/rendering.html#the-fieldset-and-legend-elements ), although the spec probably only says that due to legacy horribleness.
I disagree that the current behavior is in line with the spec. It would be in line with it if `fieldset` had `min-width: (-moz-)min-content;` by default, but the author could easily override this (as currently WebKit/Blink did). Moreover, this shouldn't break much backwards compatibility (except edge cases like one mentioned in WebKit bug referenced in Comment #18 as "parallel"). Current behavior, when we have to use non-intuitive hacks (e.g. changing `display` to `table-*`, as suggested in Comment #17) in order just to control the width of the fieldset, seems like something to be fixed.
Oh my gosh... this bug is still here?! I opened this report in 2009 and in the meantime I've also changed country of residence. :-) When I was young (born in 1966) was a common thought that in the year 2000 mankind would have been an advanced civilization traveling with UFO, dressing metallic spacesuits with antigravity boots. On the contrary, in the year 2014 we can't even deal with common standards about antediluvian stuff like CSS and HTML! :-)
Attached file workaround.htm —
For me (Win7, FF 36) wrapping the inner divs within an element styled like this { display: table; table-layout: fixed; width: 100%; } fixes the issue.
(In reply to Francesco from comment #22) > For me (Win7, FF 36) wrapping the inner divs within an element styled like > this > { > display: table; > table-layout: fixed; > width: 100%; > } > fixes the issue. There is also a different workaround in the answer to my Stack Overflow question (http://stackoverflow.com/a/17863685/578288): @-moz-document url-prefix() { fieldset { display: table-cell; } } This workaround doesn’t require creating an extra wrapper element.
(In reply to Rory O’Kane from comment #23) > (In reply to Francesco from comment #22) > > For me (Win7, FF 36) wrapping the inner divs within an element styled like > > this > > { > > display: table; > > table-layout: fixed; > > width: 100%; > > } > > fixes the issue. > > There is also a different workaround in the answer to my Stack Overflow > question (http://stackoverflow.com/a/17863685/578288): > > @-moz-document url-prefix() { > fieldset { > display: table-cell; > } > } > > This workaround doesn’t require creating an extra wrapper element. Your workaround is surely better. However, I'm wondering why 'table-layout: fixed' can be used to work around several, possibly related, Firefox bugs: for instance, bug 823483.
So in trying to understanding the list of bugs related to max-width: 100% I tested https://bug504622.bugzilla.mozilla.org/attachment.cgi?id=388957 in Firefox, Opera (Blink) and Safari and I'm getting the same results. So it seems there is interop for this one (webcompat wise) (have not tested IE).
Flags: needinfo?(bzbarsky)
Flags: needinfo?(anthony)
Looks like IE 11 has different behavior. IE Edge has the same behavior as Gecko/Blink/WebKit.
Flags: needinfo?(bzbarsky)
Thanks bz. Still in the process of understanding all the issues with regards to max-width and intrinsic sizes. I made this to help me understand, but I guess the bug linkage is messy and the dependencies are not that meaningful There might be a way to clean that up. http://la-grange.net/2015/07/02/max-width-moz/max-width
Added relevant testcase to the HTML test suite: https://github.com/w3c/web-platform-tests/pull/1969
Blocks: 1035091
I have set this to block bug 1035091 because (anecdotally) a major legitimate use of @-moz-document in author sheets is working around this bug. We generally don't intend to provide CSS constructs that let you apply rules to Gecko but nothing else, but I feel a degree of caution is warranted (especially since 1035091 had to be backed out once already for add-on fallout).
OS: Windows XP → All
Hardware: x86 → All
Blocks: 1230801
This is a major interop bug that the creators of popular frameworks have to hack around, including Bootstrap and Drupal. Let's fix this, and get Firefox to behave as other browsers do.
Keywords: DevAdvocacy
Whiteboard: [DevRel:P2]
Jen, the problem is that it's not clear what "fix" means here. For example, consider this testcase: <!DOCTYPE html> <body style="width: 0"> <fieldset style="width: 0; display: block; background: yellow"> Thisisaverylongword </fieldset> </body> This shows the fieldset not shrinking below it's min-intrinsic width in Firefox, Safari, and Chrome; I don't have IE/Edge to test with right this second. Are you suggesting we change that behavior? If not, what change are you proposing, exactly?
Flags: needinfo?(jensimmons)
Or put another way, what is there lack of interop with?
Checked Edge on that testcase, and it matches the other browsers.
Ah, I guess one behavior difference is what happens if you explicitly set "min-width: 0" on the fieldset. That seems like a reasonable behavior change we could make in Gecko. Is that what you were talking about?
Making `fieldset { min-width: 0; }` disable the weird legacy behavior, like it already does in WebKit/Blink (haven't tested Edge), is all that we at Bootstrap are asking for. Here's a testcase for your convenience: http://w3c-test.org/html/rendering/non-replaced-elements/the-fieldset-element-0/min-width-not-important.html
Flags: needinfo?(jensimmons)
Summary: Fieldsets never shrink below their min-intrinsic width → Fieldsets never shrink below their min-intrinsic width, even if min-width is explicitly set to 0
This change will allow the border drawing code to deal with the following changes, which will make us no longer force the fieldset to be wider than the legend. Without this change, doing that causes the vertical inline-start-side and inline-end-side borders of the fieldset to paint under the legend, because the current code only modifies the painting of the block-start-side border (the one the legend is positioned on). This does change behavior in one situation, which the new tests test. For relatively positioned legends, we used to use the original vertical location but the positioned horizontal location of the legend to decide which parts of the border to not paint. In the new setup, we use the original location for both. I did check that this new behavior matches Chrome and Safari. Edge seems to have our old behavior.
Attachment #8813460 - Flags: review?(matt.woodrow)
Attachment #8813460 - Flags: review?(dbaron)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
> part 1. Rewrite fieldset border drawing to just clip to the area outside the legendm Assume I fixed the typo there: "legend". ;)
I'm not sure how to write an automated test for the "inline-start/end-side border doesn't get drawn under the legend" bit that part 1 was fixing, but I did manually test it.
Attachment #8813462 - Flags: review?(dbaron)
Attachment #8813491 - Flags: review?(dbaron)
Attachment #8813462 - Attachment is obsolete: true
Attachment #8813462 - Flags: review?(dbaron)
Comment on attachment 8813460 [details] [diff] [review] part 1. Rewrite fieldset border drawing to just clip to the area outside the legendm instead of doing it in pieces with different clip rects Review of attachment 8813460 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/forms/nsFieldSetFrame.cpp @@ +249,5 @@ > + // to paint within our rect but outside the legend rect. > + RefPtr<PathBuilder> pathBuilder = > + drawTarget->CreatePathBuilder(FillRule::FILL_WINDING); > + int32_t appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel(); > + AppendRectToPath(pathBuilder, It wouldn't hurt to add a AppendCounterClockwiseRectToPath helper instead of a bool param, but the comment above is enough. @@ +258,5 @@ > + NSRectToSnappedRect(legendRect, appUnitsPerDevPixel, > + *drawTarget), > + false); > + RefPtr<Path> clipPath = pathBuilder->Finish(); > + Whitespace!
Attachment #8813460 - Flags: review?(matt.woodrow) → review+
> Whitespace! Fixed.
Yes, Boris, I believe the problem is a lack of interop on what happens with `fieldset { min-width: 0; }`. Drupal core CSS files are full of things like this: ``` .fieldgroup { min-width: 0; } /** * We've temporarily added this Firefox specific rule here to fix fieldset * widths. * @todo remove once this Mozilla bug is fixed. * See https://bugzilla.mozilla.org/show_bug.cgi?id=504622 */ @-moz-document url-prefix() { .fieldgroup { display: table-cell; } } ``` I'm glad to see some action here. Thanks. Exciting.
Comment on attachment 8813460 [details] [diff] [review] part 1. Rewrite fieldset border drawing to just clip to the area outside the legendm instead of doing it in pieces with different clip rects >Bug 504622 part 1. Rewrite fieldset border drawing to just clip to the area outside the legendm instead of doing it in pieces with different clip rects. r=mattwoodrow,dbaron Reminder to "legendm" -> "legend" >legend. Without this change, doing that causes the vertical inline-start-side Maybe change "Without this change, doing that" to be "Without this patch, allowing the fieldset to be narrower than the legend". (I misread this sentence the first time, and thought you were only applying the clip to the top border. But I agree clipping all borders sounds like the right thing.) >This does change behavior in one situation, which the new tests test. For >relatively positioned legends, we used to use the original vertical location but >the positioned horizontal location of the legend to decide which parts of the >border to not paint. In the new setup, we use the original location for both. >I did check that this new behavior matches Chrome and Safari. Edge seems to >have our old behavior. I agree that the new behavior for relatively positioned legends is correct. >+ // We set up a clip path which has our rect clockwise and the legend rect >+ // counterclockwise, with FILL_WINDING as the fill rule. That will allow us >+ // to paint within our rect but outside the legend rect. This doesn't seem like it will work right for the cases where the border extends outside the border-rect. This can happen with 'border-image-outset'. Please add a testcase for this, check that it fails with the current patch, and fix it by making the outer rect of the clip be the fieldset's visual overflow rect (but you'll need to leave |rect| alone for its other uses). r=dbaron with that
Attachment #8813460 - Flags: review?(dbaron) → review+
Comment on attachment 8813491 [details] [diff] [review] Part 2 with another reftest fix try found >I'm not sure how to write an automated test for the "inline-start/end-side >border doesn't get drawn under the legend" bit that part 1 was fixing, but I did >manually test it. Seems like something minimal would be to use a != reftest: * test and reference each have a fieldset with border-top: none but borders other sides (or maybe just a border on the right) and a fixed width, and a legend *with visibility:hidden* * the test has enough text in the legend (or, better, an inline-block with width) to obscure part of the right border, but the reference has a small legend I also wonder if it makes more sense to use &#x200b; (zero width space) rather than zero width non-joiner in fieldset-min-width-2-ref.html. r=dbaron, preferably with such a test added
Attachment #8813491 - Flags: review?(dbaron) → review+
> This can happen with 'border-image-outset'. Exciting. Note that this is broken without this patch too, as long as we have a legend, because we end up clipping to "rect" or some subset of it in all cases. So for example this testcase: <style> fieldset { width: 100px; height: 100px; border: 10px solid; border-image: url(https://www.w3.org/2014/10/stdvidthumb.png) 100%/1/1; } </style> <fieldset> <legend></legend> </fieldset> doesn't show any border images. It does if the legend is removed. I tried using "GetVisualOverflowRect() + aPt" for the clockwise rect and it makes that test pass. But the documentation for GetVisualOverflowRect() says: * @return the rect relative to this frame's origin, but after * CSS transforms have been applied (i.e. not really this frame's coordinate * system, and may not contain the frame's border-box, e.g. if there * is a CSS transform scaling it down) and indeed if I throw in "transform: scale(0.5)" then the border doesn't get painted at all. Are there cases other than border-image-outset that can cause borders to paint outside the border-rect? If not, then inflating rect by GetStyleBorder()->GetImageOutset() seems like the right thing to do. I'll post a patch doing that. > Seems like something minimal would be to use a != reftest: Actually, I found a simple way to test this given that idea. The test has: <style> fieldset { min-width: 0; width: 0; } legend { width: 100px; height: 20px; } </style> <fieldset> <legend></legend> </fieldset> and the reference is exactly the same except for "background: white" on the legend.
Attachment #8815320 - Flags: review?(dbaron)
I suppose another pragmatic solution is to simply use aDirtyRect for the "outer" clip rect. At least assuming it's in the coordinate system of our frame. That seems like it should really cover all cases...
And assuming that dirty rect is actually correct in this case...
Comment on attachment 8815320 [details] [diff] [review] Interdiff addressing part 1 review comments dbaron pointed out GetVisualOverflowRectRelativeToSelf() has the right coordinates to use for our clip.
Attachment #8815320 - Flags: review?(dbaron)
And per discussion, the reason for the random try failures with the border-image-outset is that nsDisplayFieldSetBorderBackground's GetBounds() wasn't including the images in that case. Fixed that with GetVisualOverflowRectRelativeToSelf() as well.
Attachment #8813460 - Attachment is obsolete: true
Attachment #8813491 - Attachment is obsolete: true
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/056f728704e7 part 1. Rewrite fieldset border drawing to just clip to the area outside the legend instead of doing it in pieces with different clip rects. r=mattwoodrow,dbaron https://hg.mozilla.org/integration/mozilla-inbound/rev/c024721d9b03 part 2. Allow fieldsets to shrink below their intrinsic min-width and below the width of their legend if their min-width is explicitly overridden. r=dbaron
Attachment #8815320 - Attachment is obsolete: true
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/694d5dac1dcc part 1. Rewrite fieldset border drawing to just clip to the area outside the legend instead of doing it in pieces with different clip rects. r=mattwoodrow,dbaron https://hg.mozilla.org/integration/mozilla-inbound/rev/096c7943a1c8 part 2. Allow fieldsets to shrink below their intrinsic min-width and below the width of their legend if their min-width is explicitly overridden. r=dbaron
Blocks: 424225
That was an unexpected pass because I removed the assertions that crashtest was hitting. Relanded with the test annotations changed to not claim this test asserts. I filed bug 1321127 on the fact that _only_ Android picked up the fact that the wrong number of assertions was now happening...
Flags: needinfo?(bzbarsky)
https://hg.mozilla.org/mozilla-central/rev/694d5dac1dcc https://hg.mozilla.org/mozilla-central/rev/096c7943a1c8
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1e768923183b followup for a review comment I missed. r=dbaron
> I also wonder if it makes more sense to use &#x200b; (zero width space) Er, I missed this somehow. Landing that change now.
Depends on: 1326163
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: