Bug 94825 - floated element with negative margin causes text wrap bug
Summary: floated element with negative margin causes text wrap bug
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Minor
Assignee: Robert Hogan
URL: http://stackoverflow.com/questions/12...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-23 10:00 PDT by Alex
Modified: 2012-10-21 02:44 PDT (History)
6 users (show)

See Also:


Attachments
screenshot from misbehavior (9.07 KB, image/png)
2012-08-23 10:00 PDT, Alex
no flags Details
Patch (4.90 KB, patch)
2012-10-09 13:04 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff
Patch (7.34 KB, patch)
2012-10-16 12:49 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff
Patch (6.38 KB, patch)
2012-10-16 14:39 PDT, Robert Hogan
leviw: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex 2012-08-23 10:00:51 PDT
Created attachment 160190 [details]
screenshot from misbehavior

The text will be displayed outside the surrounding container.


Except Android Browser and Chrome on Android.
Comment 1 Robert Hogan 2012-10-09 13:04:25 PDT
Created attachment 167832 [details]
Patch
Comment 2 Levi Weintraub 2012-10-15 10:49:29 PDT
Comment on attachment 167832 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167832&action=review

> Source/WebCore/ChangeLog:9
> +        Avoid over-estimating the available width on the line by ensuring that the offset taken to avoid
> +        floats on the line is at least as much as the offset given by border, margin and padding.

This only applies when we're dealing with negative margins, correct?

> LayoutTests/fast/block/float/float-on-line-obeys-container-padding.html:29
> +      <img src="resources/greenbox.png">

We have several versions of greenbox.png floating around, but not in this directory. How about just putting an inline block there?
Comment 3 Robert Hogan 2012-10-15 11:05:48 PDT
Comment on attachment 167832 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167832&action=review

>> Source/WebCore/ChangeLog:9
>> +        floats on the line is at least as much as the offset given by border, margin and padding.
> 
> This only applies when we're dealing with negative margins, correct?

Not necessarily. In the LTR case all we need is a leading float on the line to give a left-offset that is less than the combined border/margin/padding on that side. It's easiest to do this in a test with a negative margin on the float. I don't think that's the only case that can happen though. Let me come back to you on it.

>> LayoutTests/fast/block/float/float-on-line-obeys-container-padding.html:29
>> +      <img src="resources/greenbox.png">
> 
> We have several versions of greenbox.png floating around, but not in this directory. How about just putting an inline block there?

Right, the test actually depends on no image being there which is a bit sloppy.
Comment 4 Levi Weintraub 2012-10-15 11:14:40 PDT
Comment on attachment 167832 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167832&action=review

>>> Source/WebCore/ChangeLog:9
>>> +        floats on the line is at least as much as the offset given by border, margin and padding.
>> 
>> This only applies when we're dealing with negative margins, correct?
> 
> Not necessarily. In the LTR case all we need is a leading float on the line to give a left-offset that is less than the combined border/margin/padding on that side. It's easiest to do this in a test with a negative margin on the float. I don't think that's the only case that can happen though. Let me come back to you on it.

Okay, I can understand that case. I'd like to see a test case covering it. It'd be good to cover the RTL case as well for coverage.

>>> LayoutTests/fast/block/float/float-on-line-obeys-container-padding.html:29
>>> +      <img src="resources/greenbox.png">
>> 
>> We have several versions of greenbox.png floating around, but not in this directory. How about just putting an inline block there?
> 
> Right, the test actually depends on no image being there which is a bit sloppy.

I don't understand why that would matter at all. If that's the case, please explain it.
Comment 5 Robert Hogan 2012-10-16 12:49:45 PDT
Created attachment 169003 [details]
Patch
Comment 6 Robert Hogan 2012-10-16 12:50:04 PDT
Comment on attachment 167832 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167832&action=review

>>>> Source/WebCore/ChangeLog:9
>>>> +        floats on the line is at least as much as the offset given by border, margin and padding.
>>> 
>>> This only applies when we're dealing with negative margins, correct?
>> 
>> Not necessarily. In the LTR case all we need is a leading float on the line to give a left-offset that is less than the combined border/margin/padding on that side. It's easiest to do this in a test with a negative margin on the float. I don't think that's the only case that can happen though. Let me come back to you on it.
> 
> Okay, I can understand that case. I'd like to see a test case covering it. It'd be good to cover the RTL case as well for coverage.

This really does need a negative margin on the float - couldn't recreate it without it so added a note to the ChangeLog.
Comment 7 Levi Weintraub 2012-10-16 13:09:37 PDT
Comment on attachment 169003 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=169003&action=review

> Source/WebCore/ChangeLog:10
> +        tends to happen when a negative margin on the float brings its right edge back before the offset

"only tends to"? It's not useful to be intentionally vague in the ChangeLog. Can you not confirm this happens only in that case?

> LayoutTests/ChangeLog:11
> +        * fast/block/float/float-on-line-obeys-container-padding.html: Added.
> +        * fast/block/float/float-on-line-obeys-container-padding-expected.html: Added.
> +        * fast/block/float/float-on-line-obeys-container-padding-rtl.html: Added.
> +        * fast/block/float/float-on-line-obeys-container-padding-rtl-expected.html: Added.

Nit: these could be combined into one test.

> LayoutTests/fast/block/float/float-on-line-obeys-container-padding-rtl-expected.html:29
> +      <img src="../../css/resources/greenbox.png">

I thought this

> LayoutTests/fast/block/float/float-on-line-obeys-container-padding.html:29
> +      <img src="../../css/resources/greenbox.png">

I thought you said this only happened when the image wasn't there? I'm also not sure why you aren't just using an inline-block?
Comment 8 Robert Hogan 2012-10-16 14:30:04 PDT
Comment on attachment 169003 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=169003&action=review

>> LayoutTests/fast/block/float/float-on-line-obeys-container-padding.html:29
>> +      <img src="../../css/resources/greenbox.png">
> 
> I thought you said this only happened when the image wasn't there? I'm also not sure why you aren't just using an inline-block?

I meant that the test needs a float that's 25px by 25px - a missing image renderer worked fine for that purpose. So does greenbox.png.
Comment 9 Robert Hogan 2012-10-16 14:38:40 PDT
Comment on attachment 169003 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=169003&action=review

>> Source/WebCore/ChangeLog:10
>> +        tends to happen when a negative margin on the float brings its right edge back before the offset
> 
> "only tends to"? It's not useful to be intentionally vague in the ChangeLog. Can you not confirm this happens only in that case?

I can't recreate it in any other case but that doesn't mean I haven't missed a trick.

>> LayoutTests/ChangeLog:11
>> +        * fast/block/float/float-on-line-obeys-container-padding-rtl-expected.html: Added.
> 
> Nit: these could be combined into one test.

OK
Comment 10 Robert Hogan 2012-10-16 14:39:04 PDT
Created attachment 169032 [details]
Patch
Comment 11 Levi Weintraub 2012-10-17 11:34:04 PDT
Comment on attachment 169032 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=169032&action=review

> Source/WebCore/ChangeLog:10
> +        happens when a negative margin on the float brings its right edge back before the offset

Nit: this presumably effects both edges.
Comment 12 Robert Hogan 2012-10-21 02:44:50 PDT
Committed r131998: <http://trac.webkit.org/changeset/131998>