Bug 94825 – floated element with negative margin causes text wrap bug

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Alex]

Created attachment 160190 [details]
screenshot from misbehavior

The text will be displayed outside the surrounding container.


Except Android Browser and Chrome on Android.

[Robert Hogan]

Created attachment 167832 [details]
Patch

[Levi Weintraub]

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?

[Robert Hogan]

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.

[Levi Weintraub]

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.

[Robert Hogan]

Created attachment 169003 [details]
Patch

[Robert Hogan]

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.

[Levi Weintraub]

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?

[Robert Hogan]

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.

[Robert Hogan]

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

[Robert Hogan]

Created attachment 169032 [details]
Patch

[Levi Weintraub]

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.

[Robert Hogan]

Committed r131998: <http://trac.webkit.org/changeset/131998>