WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 35981
Account for margin after when laying out <legend> element
https://bugs.webkit.org/show_bug.cgi?id=35981
Summary
Account for margin after when laying out <legend> element
Patrick Paul-Hus
Reported
2010-03-10 12:46:49 PST
Created
attachment 50425
[details]
Demonstration of the bug I defined a form using semantic markup, but it seems that I can't get a bottom-margin to render under the legend element. Using the web inspector, it does say that there's a bottom-margin applied, but the content that comes after is pushed right underneath and seems to ignore the bottom-margin. I've attached a small html document which shows the issue. It renders fine in IE and FF but there's no bottom margin in Chrome Beta for and Safari 4. I am attaching a small HTML document as proof of concept.... for now I fixed the problem by using padding instead of margin.
Attachments
Demonstration of the bug
(1.90 KB, text/html)
2010-03-10 12:46 PST
,
Patrick Paul-Hus
no flags
Details
Screenshot of margins not being applied in Chromium 13 ubuntu 10.04
(54.21 KB, image/png)
2011-09-07 09:01 PDT
,
Pip
no flags
Details
Proposed fix, take the margin after into account.
(12.39 KB, patch)
2012-06-10 07:49 PDT
,
Julien Chaffraix
no flags
Details
Formatted Diff
Diff
Patch for landing
(12.43 KB, patch)
2012-06-11 15:03 PDT
,
Julien Chaffraix
no flags
Details
Formatted Diff
Diff
Patch for landing
(12.28 KB, patch)
2012-06-11 15:43 PDT
,
Julien Chaffraix
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Pip
Comment 1
2011-09-07 09:01:32 PDT
Created
attachment 106589
[details]
Screenshot of margins not being applied in Chromium 13 ubuntu 10.04 Exactly the same code in Firefox 6 makes the second legend float 50px above the fieldset. From
http://www.w3schools.com/cssref/tryit.asp?filename=trycss_margin-bottom
WebKit Review Bot
Comment 2
2011-09-07 09:04:36 PDT
Attachment 106589
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1 Total errors found: 0 in 0 files If any of these errors are false positives, please file a bug against check-webkit-style.
Octavian Damiean
Comment 3
2012-03-05 05:20:11 PST
I can confirm this bug. Here is a jsfiddle[1] demonstrating the bug. This fiddle[2] shows that the legend element gets rendered with a margin when not setting it inside of a fieldset which of course is an importer use of the legend element. [1]:
http://jsfiddle.net/mainerror/XJYgL/
[2]:
http://jsfiddle.net/mainerror/XJYgL/1/
Patrick Paul-Hus
Comment 4
2012-03-05 17:42:20 PST
2 years soon! Happy bday
Bug 35981
!
Julien Chaffraix
Comment 5
2012-06-06 21:29:57 PDT
The issue is in RenderFieldset logic that needs to account for the <legend>'s margins in the block flow direction. Note that using margin-top doesn't work either.
Julien Chaffraix
Comment 6
2012-06-10 07:49:43 PDT
Created
attachment 146736
[details]
Proposed fix, take the margin after into account.
Abhishek Arya
Comment 7
2012-06-11 11:05:33 PDT
Comment on
attachment 146736
[details]
Proposed fix, take the margin after into account. View in context:
https://bugs.webkit.org/attachment.cgi?id=146736&action=review
r=me
> Source/WebCore/ChangeLog:3 > + Can't apply a bottom-margin to the legend element
nit: probably like "Add bottom-margin support to legend element".
> Source/WebCore/rendering/RenderFieldset.cpp:108 > + LayoutUnit collapsedLegendExtend;
nit: probably s/Extend/Extent
Julien Chaffraix
Comment 8
2012-06-11 15:03:19 PDT
Created
attachment 146923
[details]
Patch for landing
Abhishek Arya
Comment 9
2012-06-11 15:06:32 PDT
Just a very friendly reminder on this cleanup in the testcase :) -moz-writing-mode: vertical-lr; writing-mode: vertical-lr;
Julien Chaffraix
Comment 10
2012-06-11 15:35:10 PDT
Comment on
attachment 146923
[details]
Patch for landing
> Just a very friendly reminder on this cleanup in the testcase :)
Right, let me remove the -moz* ones.
Julien Chaffraix
Comment 11
2012-06-11 15:43:28 PDT
Created
attachment 146936
[details]
Patch for landing
WebKit Review Bot
Comment 12
2012-06-11 16:14:54 PDT
Comment on
attachment 146936
[details]
Patch for landing Clearing flags on attachment: 146936 Committed
r120017
: <
http://trac.webkit.org/changeset/120017
>
WebKit Review Bot
Comment 13
2012-06-11 16:14:59 PDT
All reviewed patches have been landed. Closing bug.
mitz
Comment 14
2012-06-11 18:17:51 PDT
(In reply to
comment #12
)
> (From update of
attachment 146936
[details]
) > Clearing flags on attachment: 146936 > > Committed
r120017
: <
http://trac.webkit.org/changeset/120017
>
This caused fast/block/basic/fieldset-stretch-to-legend.html to fail. See for example <
http://build.webkit.org/results/Lion%20Release%20(WebKit2%20Tests)/r120021%20(8106)/results.html
>
mitz
Comment 15
2012-06-11 18:30:18 PDT
(In reply to
comment #14
)
> (In reply to
comment #12
) > > (From update of
attachment 146936
[details]
[details]) > > Clearing flags on attachment: 146936 > > > > Committed
r120017
: <
http://trac.webkit.org/changeset/120017
> > > This caused fast/block/basic/fieldset-stretch-to-legend.html to fail. See for example <
http://build.webkit.org/results/Lion%20Release%20(WebKit2%20Tests)/r120021%20(8106)/results.html
>
This test needs its expected results updated.
Julien Chaffraix
Comment 16
2012-06-11 18:31:40 PDT
(In reply to
comment #15
)
> (In reply to
comment #14
) > > (In reply to
comment #12
) > > > (From update of
attachment 146936
[details]
[details] [details]) > > > Clearing flags on attachment: 146936 > > > > > > Committed
r120017
: <
http://trac.webkit.org/changeset/120017
> > > > > This caused fast/block/basic/fieldset-stretch-to-legend.html to fail. See for example <
http://build.webkit.org/results/Lion%20Release%20(WebKit2%20Tests)/r120021%20(8106)/results.html
> > > This test needs its expected results updated.
Yes, it looks like I missed it during testing but the new results make sense. I will do the update as EFL needs the same treatment.
mitz
Comment 17
2012-06-11 18:43:13 PDT
(In reply to
comment #16
)
> (In reply to
comment #15
) > > (In reply to
comment #14
) > > > (In reply to
comment #12
) > > > > (From update of
attachment 146936
[details]
[details] [details] [details]) > > > > Clearing flags on attachment: 146936 > > > > > > > > Committed
r120017
: <
http://trac.webkit.org/changeset/120017
> > > > > > > This caused fast/block/basic/fieldset-stretch-to-legend.html to fail. See for example <
http://build.webkit.org/results/Lion%20Release%20(WebKit2%20Tests)/r120021%20(8106)/results.html
> > > > > This test needs its expected results updated. > > Yes, it looks like I missed it during testing but the new results make sense. I will do the update as EFL needs the same treatment.
I updated the Mac expected results in <
http://trac.webkit.org/r120026
>.
Julien Chaffraix
Comment 18
2012-06-11 18:49:48 PDT
> I updated the Mac expected results in <
http://trac.webkit.org/r120026
>.
And updated / skipped the rest of the platforms in
http://trac.webkit.org/changeset/120027
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug