[webkit-reviews] review canceled: [Bug 79128] Fieldset unexpectedly stretches to minimum intrinsic width. : [Attachment 132768] Updated patch with reftests
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Mar 20 10:15:21 PDT 2012
Julien Chaffraix <jchaffraix at webkit.org> has canceled SravanKumar S
<ssandela at innominds.com>'s request for review:
Bug 79128: Fieldset unexpectedly stretches to minimum intrinsic width.
https://bugs.webkit.org/show_bug.cgi?id=79128
Attachment 132768: Updated patch with reftests
https://bugs.webkit.org/attachment.cgi?id=132768&action=review
------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=132768&action=review
> Source/WebCore/rendering/RenderFieldset.cpp:201
> + // If width is exclusively specified then Fieldsets should not stretch
> + if (style()->width().isPercent())
It's undesirable to have different behavior between specified widths. If fixed
positions don't work here, I think our logic needs to be changed elsewhere so
that they work the same.
Also the fixed position case *should* be tested.
> Source/WebCore/rendering/RenderFieldset.h:45
> + virtual bool stretchesToMinIntrinsicLogicalWidth() const;
Still extra-space.
>
LayoutTests/fast/forms/fieldset-percent-width-nostretch-ifspecified-expected.ht
ml:17
> +
1111111111111111111111111111111111111111111111111111111111111111111111111111111
11111111111111111111111111111111111111111111111111111111111111111111111111
First a test should include:
* the bug title
* the bug number
* condition to pass (our container should be 50% here)
> LayoutTests/fast/forms/fieldset-percent-width-nostretch-ifspecified.html:1
> +<html>
Please add a doctype (those comments are also for the -expected.html):
<!DOCTYPE html>
> LayoutTests/fast/forms/fieldset-percent-width-nostretch-ifspecified.html:3
> + <style type="text/css" media="screen">
No need for the attribute.
> LayoutTests/fast/forms/fieldset-percent-width-nostretch-ifspecified.html:15
> + <div class="wrap">
the wrap class is unused. I would add it back and put a fixed size on this
wrapper to limit the potential for cross-browser differences.
More information about the webkit-reviews
mailing list