Avoid adding placeholder when deleting last text in root

Avoid adding placeholder when deleting last text in root

Editing code uses placeholders for all manner of different functions including
holding open content editable blocks. When there's no content in the blocks except
text, these placeholders aren't necessary and is confusing Android's IME path.

Android's IME path should probably just be fixed, but in the meantime, this
creates fewer placeholders, which is an improvement.

NB, I tried numerous other strategies to try and more aggressively remove br
placeholders in other cases where they aren't necessary, but it ended up being a
very deep rabbit hole of editing nightmares that ultimately just wasn't worth it.

BUG=222806, 236726
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=150400

[ojan, 2013-05-09 23:34:59]

lgtm

This seems fine. We can view it as an incremental step in the direction of
getting rid of unneeded placeholders.

[ojan, 2013-05-10 23:38:21]

https://codereview.chromium.org/14969020/diff/1/Source/core/editing/DeleteSel...
File Source/core/editing/DeleteSelectionCommand.cpp (right):

https://codereview.chromium.org/14969020/diff/1/Source/core/editing/DeleteSel...
Source/core/editing/DeleteSelectionCommand.cpp:805: bool
rootWillStayOpenWithoutPlaceholder = downstreamEnd.containerNode()->isTextNode()
Actually, now that I look at this more closely, why does the containerNode need
to be a textNode? One if you are deleting the last image in a contentEditable?

[leviw_travelin_and_unemployed, 2013-05-10 23:50:57]

On 2013/05/10 23:38:21, ojan wrote:
>
https://codereview.chromium.org/14969020/diff/1/Source/core/editing/DeleteSel...
> File Source/core/editing/DeleteSelectionCommand.cpp (right):
> 
>
https://codereview.chromium.org/14969020/diff/1/Source/core/editing/DeleteSel...
> Source/core/editing/DeleteSelectionCommand.cpp:805: bool
> rootWillStayOpenWithoutPlaceholder =
downstreamEnd.containerNode()->isTextNode()
> Actually, now that I look at this more closely, why does the containerNode
need
> to be a textNode? One if you are deleting the last image in a contentEditable?

You're right, though the logic would become slightly more complicated due to the
existence of new & deprecated positions; If it's not a text node, we'd want the
containerNode itself instead of the containerNode's parent. I'm going to also
take the change to HTMLInputElement in https://codereview.chromium.org/15011008/

[yosin_UTC9, 2013-05-13 03:58:45]

This patch also fixes issue 236726 and changes for test of
input-placeholder-visibility-3.html and textarea-placholder-visibility-1.html
cover it.

[leviw_travelin_and_unemployed, 2013-05-14 18:53:42]

For the record, I think this is ready to go. I'm not sure of what's up with the
Mac or Linux bots...

[ojan, 2013-05-14 19:02:17]

C++ side looks good. A few questions about the tests.

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/TestExpectations
File LayoutTests/TestExpectations (right):

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/TestExpectati...
LayoutTests/TestExpectations:1570: # crbug.com/234758 [ Debug ]
fast/parser/nested-fragment-parser-crash.html [ Pass Timeout ]
We should not put in commented out lines. They get stale and forgotten about
very quickly. Instead, you should add the extra bug line to the new entry on
line 1620 (you can have multiple bugs per line) and leave it in after the
rebaseline is done.

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/TestExpectati...
LayoutTests/TestExpectations:1620: crbug.com/222806
fast/parser/nested-fragment-parser-crash.html [ Failure Pass ImageOnlyFailure ]
Don't you need Timeout in this list as well?

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/editing/paste...
File LayoutTests/editing/pasteboard/smart-paste-in-text-control-expected.txt
(right):

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/editing/paste...
LayoutTests/editing/pasteboard/smart-paste-in-text-control-expected.txt:3: FAIL:
Smart cutting and pasting do not result in the original text.
Looks like this test needs to be updated. We're the only port now. If the test
is wrong, we should fix the test.

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/fast/parser/n...
File LayoutTests/fast/parser/nested-fragment-parser-crash-expected.txt (right):

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/fast/parser/n...
LayoutTests/fast/parser/nested-fragment-parser-crash-expected.txt:3: CONSOLE
MESSAGE: line 19: Uncaught RangeError: Maximum call stack size exceeded.
Is this really changed by this patch or is the test just flaky?

[leviw_travelin_and_unemployed, 2013-05-14 23:32:19]

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/TestExpectations
File LayoutTests/TestExpectations (right):

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/TestExpectati...
LayoutTests/TestExpectations:1570: # crbug.com/234758 [ Debug ]
fast/parser/nested-fragment-parser-crash.html [ Pass Timeout ]
On 2013/05/14 19:02:18, ojan wrote:
> We should not put in commented out lines. They get stale and forgotten about
> very quickly. Instead, you should add the extra bug line to the new entry on
> line 1620 (you can have multiple bugs per line) and leave it in after the
> rebaseline is done.

Dang, I didn't know you could do that. Thanks!

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/TestExpectati...
LayoutTests/TestExpectations:1620: crbug.com/222806
fast/parser/nested-fragment-parser-crash.html [ Failure Pass ImageOnlyFailure ]
On 2013/05/14 19:02:18, ojan wrote:
> Don't you need Timeout in this list as well?

Yes, good catch.

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/editing/paste...
File LayoutTests/editing/pasteboard/smart-paste-in-text-control-expected.txt
(right):

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/editing/paste...
LayoutTests/editing/pasteboard/smart-paste-in-text-control-expected.txt:3: FAIL:
Smart cutting and pasting do not result in the original text.
Sorry... I ran reset-results on too many tests, and this got included despite
the fact that we skip it on all platforms. We don't support smart-paste. This
test should probably just be removed, but I'll do it in a different CL. I also
wish we avoided resetting results for skipped tests.

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/fast/parser/n...
File LayoutTests/fast/parser/nested-fragment-parser-crash-expected.txt (right):

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/fast/parser/n...
LayoutTests/fast/parser/nested-fragment-parser-crash-expected.txt:3: CONSOLE
MESSAGE: line 19: Uncaught RangeError: Maximum call stack size exceeded.
On 2013/05/14 19:02:18, ojan wrote:
> Is this really changed by this patch or is the test just flaky?

It's really changed.

[ojan, 2013-05-14 23:45:15]

lgtm

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/editing/paste...
File LayoutTests/editing/pasteboard/smart-paste-in-text-control-expected.txt
(right):

https://codereview.chromium.org/14969020/diff/27001/LayoutTests/editing/paste...
LayoutTests/editing/pasteboard/smart-paste-in-text-control-expected.txt:3: FAIL:
Smart cutting and pasting do not result in the original text.
On 2013/05/14 23:32:19, Levi wrote:
> Sorry... I ran reset-results on too many tests, and this got included despite
> the fact that we skip it on all platforms. We don't support smart-paste.

Wow. That's pretty bad. We support smart-cut, but not smart-paste on Mac.

[commit-bot: I haz the power, 2013-05-15 00:14:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leviw@chromium.org/14969020/36001

[leviw_travelin_and_unemployed, 2013-05-15 00:15:08]

On 2013/05/14 23:45:15, ojan wrote:
> lgtm
> 
>
https://codereview.chromium.org/14969020/diff/27001/LayoutTests/editing/paste...
> File LayoutTests/editing/pasteboard/smart-paste-in-text-control-expected.txt
> (right):
> 
>
https://codereview.chromium.org/14969020/diff/27001/LayoutTests/editing/paste...
> LayoutTests/editing/pasteboard/smart-paste-in-text-control-expected.txt:3:
FAIL:
> Smart cutting and pasting do not result in the original text.
> On 2013/05/14 23:32:19, Levi wrote:
> > Sorry... I ran reset-results on too many tests, and this got included
despite
> > the fact that we skip it on all platforms. We don't support smart-paste.
> 
> Wow. That's pretty bad. We support smart-cut, but not smart-paste on Mac.

Yeah... to me that's worse than supporting neither :(

[commit-bot: I haz the power, 2013-05-15 02:22:05]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[commit-bot: I haz the power, 2013-05-15 04:10:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leviw@chromium.org/14969020/36001

[commit-bot: I haz the power, 2013-05-15 12:09:55]

Change committed as 150400

[Stephen White, 2013-05-15 17:07:16]

This seems to be causing crashes and flaky failures on the test
editing/undo/orphaned-selection-crash-bug32823-2.html:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=edi...