Created attachment 206173 [details] screen shot of problem. Link test doesn't wrap. REGRESSION: Links in certain twitter postings don't warp correctly on page. I'm seeing this on several of twitter posting such as Walt Mossberg and Fox News. This first occurs with WebKit r152405 but this worked on previous Webkit NB r152225. I have a reduced test case I'm working one now. 1) Launch WebKit r152405 and signing to your Twitter account 2) Scroll down list of twitter posting and look for "Walt Mossberg - FCC Approves SoftBank Takeover of Sprint, Clearwire ". Notice the link doesn't wrap down.
Created attachment 206177 [details] attaching sample test page of twitter issue
Here is what the HTML structure of the test case: <!DOCTYPE html> <head> <title>Twitter</title> <style type="text/css"> .content-main{float:right;width:522px} </style> </head> <body> <div class="content-main"> <div> </div> <div> <p>FCC Approves SoftBank Takeover of Sprint Clearwire <a href="/DelRey">@DelRey</a> <a href="http://t.co/B9okl3g4kk"> dthin.gs/13q6ZYi<span></span><span> </span> </a> </p> </div> </html>
regressed at http://trac.webkit.org/changeset/152313 empty inlines should not affect line-wrapping: ID 25638
(In reply to comment #3) > regressed at http://trac.webkit.org/changeset/152313 > > empty inlines should not affect line-wrapping: ID 25638 bug 25638
Created attachment 206229 [details] test reduction
Created attachment 206230 [details] test reduction
Created attachment 206250 [details] Patch
zalan: rhogan: do you have a test case for this? -> http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp#L2910 zalan: rhogan: wondering as it introduced regression and the testcase that came with the commit passes even when this line is removed. ------ // If we are an empty inline in the middle of a word and don't fit on the line then clear any line break we have and find // one in the following text instead. if (!canPlaceOnLine && !canBreakHere && isEmptyInline(current.m_obj)) lBreak.clear(); ---------- It would be great to see a test case for this^ behaviour. Sadly, the comment does not say why it is done this way. As a side note, even with this regression, the line should break at a latter position. I'll figure it out why it does not.
rhogan: zalan: your findings match mine - when I wrote the patch that line was required to pass the test unless I was careless, which is also possible rhogan: zalan: so yes, that line is superfluous and can be removed - I must have gotten confused while testing and convinced myself it was required
Any update when this fix is landing ?
(In reply to comment #10) > Any update when this fix is landing ? will ping Dave again to take a look at it.
Created attachment 206871 [details] Patch
alternatively, If we prefer breaking the line after the empty inline, we should fix it like this: if (!canPlaceOnLine && !canBreakHere && isEmptyInline(current.m_obj)) - lBreak.clear(); + lBreak.m_pos = 0; else if (canPlaceOnLine && canBreakHere) so that we can still break on the line. (as right now, with lBreak.clear(), we not only clear the undesired position, but also the m_obj which confuses the breaking logic at this point)
Created attachment 206872 [details] linebreaks 1. line break before the empty inline (the current patch does this -match FF and Opera) 2, line break after the empty inline (with the alternative lBreak.m_pos = 0; change) 3, trunk behaviour, no break at all.
Comment on attachment 206871 [details] Patch r=me
<rdar://problem/14524323>
Comment on attachment 206871 [details] Patch Clearing flags on attachment: 206871 Committed r153061: <http://trac.webkit.org/changeset/153061>
All reviewed patches have been landed. Closing bug.