1102906 - Contenteditable: Drag and drop locks caret (also affected CKEditor and TinyMCE as well as Thunderbird)

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Piotr Jasiun]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.65 Safari/537.36

Steps to reproduce:

1. Create a contenteditable element with an image:
	<div contenteditable="true">
		Foo<img src="https://mozorg.cdn.mozilla.net/media/img/home/voices/firefox-logo-wordmark-white.png">bar
	</div>
2. Drag and drop the image


Actual results:

Cannot move cursor using keyboard. Even if I move cursor to the other place with a mouse I am still not able to move it with keyboard arrows.

Does not work on Firefox 33.1.1. On Firefox 30 everything is fine.

Tested on Windows 7, Ubuntu 12.04 and OS X 10.10.1.


Expected results:

Should work as expected, as in Firefox 30.

Comment 1

[Piotrek Koszuliński]

This bug affects CKEditor - http://dev.ckeditor.com/ticket/12697

Comment 2

[Alice0775 White]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/04dd691d5f59
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140602230732
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d2b67fc93f7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140603000934
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=04dd691d5f59&tochange=1d2b67fc93f7

Regressed by: Bug 924692

Comment 3

[Alice0775 White]

In local build
Last Good: a1582fd3bf01
First Bad: 1d2b67fc93f7
Regressed by 1d2b67fc93f7	Phoebe Chang — Bug 924692 - Part 4: Hooks up event handling to TouchCaret; r=roc, bugs

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

I can reproduce this issue on Nightly on non-e10s tab due to drag & drop does not yet support in bug 936092. The causes of this might be related to line 879 ~ 882 added in bug 924692 part 4 [1].

[1] http://hg.mozilla.org/mozilla-central/annotate/c9cfa9b91dea/editor/libeditor/nsEditorEventListener.cpp#l879

Comment 5

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Fix immovable caret after drag-and-drop. (v1)

The deleted lines were added in bug 924692 part 4. It will incorrectly
disable the caret in PresShell so that the caret cannot be moved by
using keyboard in nsSelectMoveScrollCommand::DoCommand().

Comment 6

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=14fd9aae2387

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8527275 [details] [diff] [review]
Fix immovable caret after drag-and-drop. (v1)

Review of attachment 8527275 [details] [diff] [review]:
-----------------------------------------------------------------

Please add a test!

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

Sadly, as we're building 34 RC today, I think it's too late to take this fix. We will ship the issue in 34 but I would like to see this fixed in 35. I am also tracking for 34 as a potential ride-along in a point release.

Comment 9

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Fix immovable caret after drag-and-drop. r=roc (v2)

Add a test.

Try result:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=28356382da60

Comment 10

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

My test runs in M-oth on try.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8528886 [details] [diff] [review]
Fix immovable caret after drag-and-drop. r=roc (v2)

Review of attachment 8528886 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/libeditor/tests/test_bug1102906.html
@@ +43,5 @@
> +    }
> +    sendChar("!");
> +
> +    is(content.textContent, "!" + textContentAfterDrop,
> +       "The exclaimation mark should be inserted at the front.");

exclamation

Comment 12

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Fix immovable caret after drag-and-drop. (v3, carry r+)

Fix typo.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e70f4f5c543f

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e70f4f5c543f

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

Can we get an uplift nomination here for Beta?  If it's low risk to take, we can put it in Monday Dec 22 Beta build, otherwise wontfix and it can go out with FF 36.

Comment 16

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

I think the risk is low. The regression was introduced in bug 924692 which added touch caret feature on B2G. By removing the hunk, the behavior of drag and drop restored to it was before bug 924692 is landed.

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

We're too late for 35 with the last beta out the door. This will have to ride to release from 36.

Comment 18

[Jean-Yves Perrier [:teoli]]

Removing dev-doc-needed: we usually don't document regression. Re-add with a kind explanation if you disagree :-)

Comment 19

[Kohei Yoshino]

I mention major regressions on the site compatibility documents. Since this has affected popular rich text editors, I'll mention this later today.

Comment 20

[Jean-Yves Perrier [:teoli]]

Oh cool! Thanks!