I think this is more of an implementation detail.

There's a difference between plain text and HTML. For the <div> in your example, while its inner HTML is " text hi", its inner plain text is "text h".

InputMethodController basically doesn't care about HTML. It counts offets only on plain text. For example, IMC::setCompositionFromExistingText obtains the range to be marked with PlainTextRange::createRange (see L655 of IMC.cpp).

So controller().setCompositionFromExistingText(emptyUnderlines, 6, 8) sets composition on "i", which is as expected.

Right.  This is working as intended, spaces are just nothings in certain contexts in HTML, and a contenteditable is normal HTML.  An NBSP character would be counted here, which is why Blink has special logic to replace spaces inputted by IME/keyboard with NBSP at start and end of contenteditables.  You may want to write your unit test with an NBSP.

BTW, I agree this is strange, but it's just one of the many reasons why contenteditable is weird/probably was a bad idea to introduce to begin with, but we're stuck with it.

Per the discussion here:
https://codereview.chromium.org/2692093003/diff/220001/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp#newcode1317

This also occurs in a test case where we have the text:
"Initial text "
and delete "Initial", which @xiaochengh says should result in the space after "Initial" (now at the beginning of the string) being replaced by an NBSP. So his assessment is that there may be a bug in commitText() not properly replacing the space with an NBSP.

Changed to mark "Available" since I would like to continue discuss regarding to Layout NG.

Since Layout NG changes whitespace handling in NGTextFragment, editing needs to adopt it. In this adaptation, we have a chance to change current behavior.

Ok here's what I figured out.

I think we were right when we said the test case I have at the top isn't really a bug, and also when we said there's a bug with commitText().

This rev changed the delete path my WhitespaceFixupAroundMarker tests in https://codereview.chromium.org/2692093003 were going through:
https://codereview.chromium.org/2675363003

I'd written https://codereview.chromium.org/2692093003 because WhitespaceFixupAroundMarker tests 2 through 4 were failing on master. After that other CL changed the delete path, we actually no longer needed the changes in my CL to update the markers properly, apparently because it goes through a different code path for replacements (I haven't looked into it too deeply yet). However, this other CL *also* makes the delete operations in my test cases not call DeleteSelectionCommand::fixupWhitespace(), causing commitText() to not count the space at the beginning (before this CL, the space gets converted to an NBSP).

So I think https://codereview.chromium.org/2675363003 introduced a regression and should probably be reverted until we can fix this bug.

Clarification: https://codereview.chromium.org/2675363003 makes the WhitespaceFixupAroundMarker tests pass because it's no longer doing the whitespace fixup, not because it's actually doing anything to improve the replacement logic.

https://crrev.com/2675363003 was partially reverted due to issue 693481 (see https://crrev.com/2711593003). 

Can you help confirm if it's still affecting your change?

Thanks.

It looks like with your revert I'm now seeing the expected behavior. I'm going to close this.

@yosin, I think we should find somewhere else to discuss potential changes for Layout NG