Bug 99194 – Regression r130057: incorrect block pref width for alternating InlineFlow and inline Replaced

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Levi Weintraub]

http://trac.webkit.org/changeset/130057 fixed block pref width calculation, but introduced a regression where alternating InlineFlows and inline Replaced elements will result in the prefs widths calculation never breaking the line.

[Levi Weintraub]

Created attachment 168466 [details]
Patch

[Levi Weintraub]

Upstream chromium bug http://code.google.com/p/chromium/issues/detail?id=154797

[Arpita Bahuguna]

Created attachment 168552 [details]
Testcase.html

Apologize for not having verified all the scenarios and for causing this regression.
I tried to come up with similar combinations which might regress the previous fix and in the process found one more issue.

The case is when a RenderInline containing some text follows another RenderInline wrapping a RenderReplaced. For this case we are not breaking the line after the RenderInline containing the text. This causes it to display extra space after the line containing the RenderReplaced.
Have uploaded the testcase containing two variations of the same issue.
FF too does not seem to handle the second variation in the uploaded testcase.

This issue occurs even after applying this patch. I think I may have a fix for the same. Should this case too be included as part of this regression or shall I raise another bug for it?

[Arpita Bahuguna]

Hey Levi,

I was trying to fix the issue mentioned in my previous comment (#3) which resulted in handling of some other failing scenarios (arising out of the fix for #3 issue) such as :

<div style="width: 50px">
<span style="float: left; border: 1px solid black;">
<span>FF doesn't handle this case.</span>Some text outside of span<span><img src="50x50.gif"/></span>
</span>
</div>

Wherein a text followed by another text and then a RenderReplaced (enclosed within a RenderInline) causes the text to overflow.

However, all these issues are independent of your patch.
 
Am trying to fix all these scenarios and if it's okay with you, will try to submit a patch for them by tomorrow.

[Levi Weintraub]

(In reply to comment #4)
> Am trying to fix all these scenarios and if it's okay with you, will try to submit a patch for them by tomorrow.

Of course I welcome your patch. In the meantime, I'd love to get this patch reviewed or the original reverted as this is causing painful regressions.

[Levi Weintraub]

Anyone? This is a definite improvement from current behavior. We didn't cover all the cases prior to r130057, and won't with this fix, but at least we'll correct this regression.

[Eric Seidel (no email)]

Comment on attachment 168466 [details]
Patch

OK.

[Levi Weintraub]

Comment on attachment 168466 [details]
Patch

Thanks Eric.

[Levi Weintraub]

Arpita, please open a new bug and add your test case (and patch when you have it) to it.

[WebKit Review Bot]

Comment on attachment 168466 [details]
Patch

Clearing flags on attachment: 168466

Committed r131359: <http://trac.webkit.org/changeset/131359>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.