Bug 47806 - Text inserted at the collapsed selection between two BRs following a non-wrapping text that forces overflow
Summary: Text inserted at the collapsed selection between two BRs following a non-wrap...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://marijn.haverbeke.nl/webkit_scr...
Keywords: HasReduction, NeedsRadar
Depends on:
Blocks:
 
Reported: 2010-10-18 03:03 PDT by Marijn Haverbeke
Modified: 2011-04-11 13:38 PDT (History)
11 users (show)

See Also:


Attachments
demo (385 bytes, text/html)
2010-11-04 00:20 PDT, Ryosuke Niwa
no flags Details
demo without editing command (389 bytes, text/html)
2010-11-04 17:59 PDT, Ryosuke Niwa
no flags Details
Test case using a timeout (322 bytes, text/html)
2010-11-05 14:05 PDT, Levi Weintraub
no flags Details
reduced slightly more (279 bytes, text/html)
2010-11-05 18:21 PDT, James Robinson
no flags Details
adds a regression test (2.40 KB, patch)
2011-04-11 11:14 PDT, Ryosuke Niwa
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marijn Haverbeke 2010-10-18 03:03:45 PDT
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.
Comment 1 Ryosuke Niwa 2010-11-03 16:36:31 PDT

*** This bug has been marked as a duplicate of bug 23474 ***
Comment 2 Marijn Haverbeke 2010-11-04 00:03:20 PDT
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?
Comment 3 Ryosuke Niwa 2010-11-04 00:20:06 PDT
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.
Comment 4 Levi Weintraub 2010-11-04 16:06:52 PDT
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...
Comment 5 Levi Weintraub 2010-11-04 17:30:27 PDT
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.
Comment 6 Ryosuke Niwa 2010-11-04 17:35:16 PDT
+dhyatt, mitz, & darin since this is a rendering issue.
Comment 7 Ryosuke Niwa 2010-11-04 17:59:50 PDT
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.
Comment 8 Levi Weintraub 2010-11-05 14:05:57 PDT
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.
Comment 9 James Robinson 2010-11-05 14:51:04 PDT
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.
Comment 10 James Robinson 2010-11-05 18:21:18 PDT
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.
Comment 11 Ojan Vafai 2011-04-10 23:29:07 PDT
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?
Comment 12 Ryosuke Niwa 2011-04-11 10:45:54 PDT
(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.
Comment 13 Ryosuke Niwa 2011-04-11 11:14:34 PDT
Created attachment 89033 [details]
adds a regression test
Comment 14 Ryosuke Niwa 2011-04-11 13:38:21 PDT
Committed r83487: <http://trac.webkit.org/changeset/83487>