Bug 47806 – Text inserted at the collapsed selection between two BRs following a non-wrapping text that forces overflow

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

[Marijn Haverbeke]

The URL I linked shows the problem more precisely, but basically, if you have

 - A designMode document
 - A line in this document that's wider than the window, not wrapping, and that's causing a horizontal scrollbar
 - A double BR afer that line
 - The cursor sitting on the blank line produced by those BRs

... then you can not type anything. Regular keyboard input seems to be ignored. Moving to another line still works, as does enter (kind of).

I'm seeing this in current versions of both Safari and Chrome, on both Linux and Windows. The issue came up because people had trouble using CodeMirror (http://codemirror.net), which creates such BR-filled documents in editable frames.

[Ryosuke Niwa]

*** This bug has been marked as a duplicate of bug 23474 ***

[Marijn Haverbeke]

Hold on. This is not a duplicate of bug 23474 at all. I submitted both, so I should know. They manifest themselves in completely different forms. Could someone please either explain how they are the same?

[Ryosuke Niwa]

Created attachment 72912 [details]
demo

(In reply to comment #2)
> Hold on. This is not a duplicate of bug 23474 at all. I submitted both, so I should know. They manifest themselves in completely different forms. Could someone please either explain how they are the same?

Ugh... sorry about that.  That was a mistake.  I think I got confused when I was back&forth between two bugs and thought they were the same bug.  Clearly, this one is different from the bug 23474.  I'll attach a demo so that we don't make the same mistake in the future.

[Levi Weintraub]

The text actually gets inserted into the DOM, but not rendered. Entering text in the same position then smashes the previous contents and replaces it with the new text. Continuing to evaluate...

[Levi Weintraub]

This seems to be a layout problem. The PRE RenderBlock has three InlineFlowBoxes associated with it, as one would expect, however the second InlineFlowBox contains only the BR that follows the "PASS" text node. The PASS node then gets skipped over in the rendering code despite having a renderer.

[Ryosuke Niwa]

+dhyatt, mitz, & darin since this is a rendering issue.

[Ryosuke Niwa]

Created attachment 73019 [details]
demo without editing command

It turned out that this is nothing to do with contenteditable.  I can reproduce the problem without execCommand / designmode="on".  However, we do need to have the selection set at the location we're inserting the text node.

[Levi Weintraub]

Created attachment 73115 [details]
Test case using a timeout

It turns out selection isn't the culprit! Setting the selection delays the DOM modification until after the first layout, which is the fail case. RenderBlock::layoutInlineChildren does the proper thing when presented with the final DOM on a "full layout" which occurs on the first layout, but subsequent layouts ignore the second InlineFlowBox, perhaps because it's not reporting that it's dirty? Adding a test case that uses a timeout instead of setting selection.

[James Robinson]

After the page has stabilized the <pre> has three RenderText children:

      RenderBlock 0x7fffeaaad018       	PRE	0x7fffeaaaed80 STYLE=width: 35ex; height: 10em; overflow: scroll;
        RenderText 0x7fffeaaaec98      	#text	0x7fffeaa95720 "You should see 'PASS' below: ----------"
        RenderBR 0x7ffff7ed91c8        	BR	0x7fffeaaaec00
        RenderText 0x7fffeb5bfb98      	#text	0x7fffeb5a17e0 "hello"
        RenderBR 0x7ffff7ed9138        	BR	0x7fffeaaaeb80
        RenderText 0x7fffeaaaeb18      	#text	0x7fffeaa956c0 " \n"

that correspond to three inline boxes in its linebox list:

$26 = {<WebCore::InlineBox> = {_vptr.InlineBox = 0x3cae730, m_next = 0x0, m_prev = 0x0, m_parent = 0x0, m_renderer = 0x7fffeaaad018, m_x = 0, m_y = 0, m_logicalWidth = 312, m_firstLine = true, 
    m_constructed = true, m_bidiEmbeddingLevel = 0 '\000', m_dirty = false, m_extracted = false, m_hasVirtualLogicalHeight = false, m_isHorizontal = true, m_endsWithBreak = false, 
    m_hasSelectedChildren = false, m_hasEllipsisBoxOrHyphen = false, m_dirOverride = false, m_isText = false, m_determinedIfNextOnLineExists = false, m_determinedIfPrevOnLineExists = false, 
    m_nextOnLineExists = false, m_prevOnLineExists = false, m_toAdd = 0, m_hasBadParent = false}, m_overflow = {<WTFNoncopyable::Noncopyable> = {<WTF::FastAllocBase> = {<No data fields>}, <No data fields>}, 
    m_ptr = 0x0}, m_firstChild = 0x7fffeaa3d248, m_lastChild = 0x7fffeaa3d248, m_prevLineBox = 0x0, m_nextLineBox = 0x7fffeb5ef318, m_includeLogicalLeftEdge = false, m_includeLogicalRightEdge = false, 
  m_hasTextChildren = true, m_hasBadChildList = false}
(gdb) p *$.m_nextLineBox 
$27 = {<WebCore::InlineBox> = {_vptr.InlineBox = 0x3cae730, m_next = 0x0, m_prev = 0x0, m_parent = 0x0, m_renderer = 0x7fffeaaad018, m_x = 0, m_y = 16, m_logicalWidth = 0, m_firstLine = false, 
    m_constructed = true, m_bidiEmbeddingLevel = 0 '\000', m_dirty = false, m_extracted = false, m_hasVirtualLogicalHeight = false, m_isHorizontal = true, m_endsWithBreak = true, 
    m_hasSelectedChildren = false, m_hasEllipsisBoxOrHyphen = false, m_dirOverride = false, m_isText = false, m_determinedIfNextOnLineExists = false, m_determinedIfPrevOnLineExists = false, 
    m_nextOnLineExists = false, m_prevOnLineExists = false, m_toAdd = 0, m_hasBadParent = false}, m_overflow = {<WTFNoncopyable::Noncopyable> = {<WTF::FastAllocBase> = {<No data fields>}, <No data fields>}, 
    m_ptr = 0x0}, m_firstChild = 0x7fffeaa3d1d8, m_lastChild = 0x7fffeaa3d1d8, m_prevLineBox = 0x7fffeb5ef3d8, m_nextLineBox = 0x7fffeb5ef258, m_includeLogicalLeftEdge = false, 
  m_includeLogicalRightEdge = false, m_hasTextChildren = true, m_hasBadChildList = false}
(gdb) p *$.m_nextLineBox 
$28 = {<WebCore::InlineBox> = {_vptr.InlineBox = 0x3cae730, m_next = 0x0, m_prev = 0x0, m_parent = 0x0, m_renderer = 0x7fffeaaad018, m_x = 0, m_y = 32, m_logicalWidth = 8, m_firstLine = false, 
    m_constructed = true, m_bidiEmbeddingLevel = 0 '\000', m_dirty = false, m_extracted = false, m_hasVirtualLogicalHeight = false, m_isHorizontal = true, m_endsWithBreak = true, 
    m_hasSelectedChildren = false, m_hasEllipsisBoxOrHyphen = false, m_dirOverride = false, m_isText = false, m_determinedIfNextOnLineExists = false, m_determinedIfPrevOnLineExists = false, 
    m_nextOnLineExists = false, m_prevOnLineExists = false, m_toAdd = 0, m_hasBadParent = false}, m_overflow = {<WTFNoncopyable::Noncopyable> = {<WTF::FastAllocBase> = {<No data fields>}, <No data fields>}, 
    m_ptr = 0x0}, m_firstChild = 0x7fffeaa3d168, m_lastChild = 0x7fffe6dfea28, m_prevLineBox = 0x7fffeb5ef318, m_nextLineBox = 0x0, m_includeLogicalLeftEdge = false, m_includeLogicalRightEdge = false, 
  m_hasTextChildren = true, m_hasBadChildList = false}


I'm not sure why the second line box has a m_logicalWidth of 0.  If I load up a similar page with the same contents as static HTML rather than dynamically created the second linebox has a m_logicalWidth of 40 which seems like a much more reasonable value.

I don't know enough about the line breaker to debug much more usefully.

[James Robinson]

Created attachment 73154 [details]
reduced slightly more

Reduced it a bit more.  This doesn't appear to be a recent regression, based on testing a few old builds.  It does seem like pretty bad behavior, though.

[Ojan Vafai]

The demo and the reduced test case seem to work in chrome 12.0.725.0 (Official Build 80304) dev at webkit revision 82765 (as in, they print out PASS).

So, this bug just needs to be converted into a layout test, unless this is already covered by existing tests. Ryosuke, do you know?

[Ryosuke Niwa]

(In reply to comment #11)
> So, this bug just needs to be converted into a layout test, unless this is already covered by existing tests. Ryosuke, do you know?

I think we can just check in jamesr's test case. Sadly, this test needs to be a render tree dump.

[Ryosuke Niwa]

Created attachment 89033 [details]
adds a regression test

[Ryosuke Niwa]

Committed r83487: <http://trac.webkit.org/changeset/83487>