New issue
Advanced search Search tips

Issue 731000 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Backspace from a text paragraph after a table ends up changing content of the table's last cell

Project Member Reported by jfernan...@igalia.com, Jun 8 2017

Issue description

Chrome Version:  60.0.3095.5
OS: (Linux)

What steps will reproduce the problem?
(1) Load the attached test case
(2) Put the cursor between 'T' and 'e' in "Testing"
(3) Hit backspace twice

What is the expected result?

Caret doesn't move up to the table's content and become nop 

What happens instead?

The caret is placed inside the last cell of the table above, deleting
cell's content and moving the rest of the paragraph's text into the table cell.

This is inconsistent with Firefox's and Edge/IE's behavior, which consider backspace a nop when reaching the start the paragraph.

 
Well, IE/Edge deletes the whole table, so it also behaves differently to Firefox.

Comment 2 by yosin@chromium.org, Jun 8 2017

Labels: Needs-Feedback
NextAction: 2017-06-22
Status: Unconfirmed (was: Untriaged)
Hi jfernandez@, could you attach a file mentioned in step 1 in #c1?
Thanks in advance.
This issue is probably related, if not the same, to https://webkit.org/b/35364
Attached test case.
backspaceIntoTable.html
120 bytes View Download
Project Member

Comment 5 by sheriffbot@chromium.org, Jun 8 2017

Cc: yosin@chromium.org
Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "yosin@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by yosin@chromium.org, Jun 9 2017

Cc: -yosin@chromium.org
NextAction: ----
Status: Available (was: Unconfirmed)
It seems we should have the spec for behavior.
Could you file the issue in https://github.com/w3c/editing/issues to get consensus of the behavior?

Thanks in advance!

P.S. I'm also a member of w3c editing task force.
 

When I tried on Chrome,
- First backspace: removed "T" then move caret before "e"
- Second backspace: Move "esting" after "2"

When I tried on Edge,
- First backspace: removed "T" then move caret after "2"
- Second backspace: remove "2"

When I tried on Firefox:
- First backspace: removed "T" then move caret before "e"
- Second backspace: Does nothing


Cc: yosin@chromium.org
Owner: jfernan...@igalia.com
Cc: -yosin@chromium.org
Cc: yosin@chromium.org
I think this is clear bug. Any feedback about how we should proceed ? There is not much feedback, so far, in the Editing Task Force github. 

In my opinion, independently of what's the expected behavior when the use press several backspaces in a line below a table (either deleting the table above or not), merging text that wasn't defined inside a table it's very weird. 
Hi Yosin, this issue is stalled and I think the bug is clear. I commented on the Editing github issue, trying to re-active the discussion towards a consensus on the right approach to implement. Perhaps you can contribute there to the discussion. 

For the time being, I'm considering just stop the backspace at the beginning of the paragraph, just like Firefox does. This would prevent not only merging the contents (bug), but deleting the whole table (feature ?)

The patch I've got now is pretty simple, so if I don't detect regressions it'd be easy to review and it's likely that it'll be accepted in WebKit.

 
IT's worth mentioning that we have a test precisely to ensure no regressions on the behavior we are trying to remove in this bug:

* editing/deleting/5032066.html

"This tests deleting when the caret is at the start of a paragraph just after a table.  The content in that paragraph should be moved into the last table cell unless that content is another table."

There is also this test, which as far as I understood it, it expects merging content into the table above:

* editing/deleting/backspace-merge-into-block.html

Status: Started (was: Available)
@yosin could you give your opinion in the ongoing discussion about this issue ? 

https://github.com/w3c/editing/issues/164 

It seems that Ryosuke is reluctant to do the change, based on some macOS Editors currently exposing such behavior. 

However, that doesn't seem a common behavior for other text editors. We may want to change our behavior even if WebKit doesn't follow us. What do you think ? 

Comment 15 by yosin@chromium.org, Mar 26 2018

Demo page: https://jsfiddle.net/hdjtvfky/6/

It seems "merging to last cell" is macOS specific behavior.
Blink's policy is follow the platform. So, we want to switch behavior based on
platform.

In this case, macOS = merge, others = not merge follow MS Word.
Because of most Windows users familiar with MS Word. Following MS Word way makes
users not surprised.
Project Member

Comment 16 by bugdroid1@chromium.org, Mar 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce922e757f5badefe2337029520926bd2d3e5e77

commit ce922e757f5badefe2337029520926bd2d3e5e77
Author: Javier Fernandez <jfernandez@igalia.com>
Date: Wed Mar 28 14:03:57 2018

Backspacing shouldn't merge content into tables except for macOS

When uses hits backspace at the start of a paragraph, the content in
that block is merged in the previous block in the paragraph above. All
the browsers have the same behavior except in the case of such previous
block being a table.

Firefox and Edge both avoid merging the content into the table. I've
filed a issue in the Editing TF [1] to get some consensus, but it seems
that current behavior was implemented to match TextEdit and other native
text editing apps in macOS.

Considering that both MS Word and Google Docs avoid merging content on
backspacing, I think it's a good option for the end user that Chrome
implements that behavior as well, improving also the browsers
interoperability (we'll match Firefox and Edge).

I'll keep the current logic for macOS using an EditingBehavior flag, so
we can switch to a different behavior based on platform.

[1] https://github.com/w3c/editing/issues/164

Bug:  731000 
Change-Id: Ibc5cad576b7115877a36929f717ff5ed194d0977
Reviewed-on: https://chromium-review.googlesource.com/981137
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546475}
[delete] https://crrev.com/d87bfe70381a444b37edfce46bd7aca0077756ec/third_party/WebKit/LayoutTests/editing/deleting/5032066-expected.txt
[modify] https://crrev.com/ce922e757f5badefe2337029520926bd2d3e5e77/third_party/WebKit/LayoutTests/editing/deleting/5032066.html
[modify] https://crrev.com/ce922e757f5badefe2337029520926bd2d3e5e77/third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block.html
[delete] https://crrev.com/d87bfe70381a444b37edfce46bd7aca0077756ec/third_party/WebKit/LayoutTests/editing/deleting/delete-block-table-expected.txt
[modify] https://crrev.com/ce922e757f5badefe2337029520926bd2d3e5e77/third_party/WebKit/LayoutTests/editing/deleting/delete-block-table.html
[modify] https://crrev.com/ce922e757f5badefe2337029520926bd2d3e5e77/third_party/WebKit/Source/core/editing/EditingBehavior.h
[modify] https://crrev.com/ce922e757f5badefe2337029520926bd2d3e5e77/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp

Status: Fixed (was: Started)
This issue should be FIXED now.

Sign in to add a comment