1256952 - [e10s] Caret breaks in contenteditable elements when I drag anything over them (without dropping)

Status: REOPENED

(Core :: DOM: Editor, defect, P2)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 48, 32bit, ID 20160313030418
STR:
 [Read Note first]
1. Open attachment 8721034 [details] (from bug 1249444)
2. Hover mouse over the identity block in urlbar, hold left mouse button
3. Move mouse in a straight line, like shown on screenshot.
4.A) Hover mouse over an empty place on Windows Taskbar, then release left mouse button
4.B) Hover mouse over some application's button on Taskbar to open it, then drop url there
4.C) Move mouse over "new tab" button and release left mouse button
5. Click on 2 different [contenteditable] elements on the page

AR:
 Caret is displayed in Step 4 to show where you're going to drop the url.
 But it doesn't disappear after Step 4.
 In Step 5 caret is still disaplayed on its old position, not where I actually click.

ER:  
 After Step 4 caret should disappear
 After Step 5 caret should appear where I clicked a [contenteditable] element

Note:
 This bug is most likely the same as bug 1233612.

Comment 2

[Benjamin Smedberg]

Neil, could you take this?

Comment 3

[Neil Deakin]

I suspect this caused by the same issue as 1256162, which I will have a fix for soon.

Comment 4

[Neil Deakin]

OK I could never reproduce this but can we check if 1256162 fixed this?

Comment 5

[arni2033]

Scenario (C) is not fixed.

Comment 6

[Neil Deakin]

Attachment: Fire a dragexit event when leaving the browser

A dragexit isn't being fired when moving outside the browser frame.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8735559 [details] [diff] [review]
Fire a dragexit event when leaving the browser

Oh, the method is a hack. Doing manual nsIFrame::HandleEvent calls and what not.
That relevant code is old, from CVS era (just moved a bit later).

Couldn't you use 
aDragEvent.AssignDragEventData(remoteEvent, true); here?
With that (or explain why you don't want to use that)

Comment 8

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa65acd9650aa99e5452dd7344ff06d387c6e3f9
Bug 1256952, send a dragexit at remote process when leaving the remote frame, r=smaug

Comment 9

[Carsten Book [:Tomcat]]

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

Comment 10

[Chris Peterson [:cpeterson]]

Comment on attachment 8735559 [details] [diff] [review]
Fire a dragexit event when leaving the browser

Approval Request Comment
[Feature/regressing bug #]: e10s
[User impact if declined]: Caret in contenteditable disappears because drag events to not fire.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: bug 1256162 should go along with it.
[String/UUID change made/needed]: None

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello Arni2033, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 12

[arni2033]

Fixed on:   Win7_64, Nightly 48, 32bit, ID 20160408030212

I can still reproduce other e10s-independent caret bugs in [contenteditable], but this one was especially bad, as it couldn't be easily worked around by clicking in another place. It's fixed now.

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8735559 [details] [diff] [review]
Fire a dragexit event when leaving the browser

Fix was verified, Aurora47+

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/86fe93b28b17

Comment 15

[Carsten Book [:Tomcat]]

backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=2334077&repo=mozilla-aurora

Comment 16

[Neil Deakin]

Attachment: Aurora version of the fix

Comment 17

[Neil Deakin]

Attachment: Aurora version of the fix

Comment 18

[Neil Deakin]

Attachment: Aurora version of the fix

One last time.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6a7001e456f1

Comment 20

[Alice0775 White]

This causes ux regression problem.
I think this should be bucked out from aurora47 before it become beta at least

Comment 21

[Neil Deakin]

(In reply to Alice0775 White from comment #20)
> This causes ux regression problem.
> I think this should be bucked out from aurora47 before it become beta at
> least

Yes.

Comment 22

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Chris, Jim, please see comment 20. Alice0775 White suggests we backout this fix from 47 before it hits Beta due to UX regressions. I believe it must be due to bug 1265966 and 1266435. Thoughts?

Comment 23

[Chris Peterson [:cpeterson]]

Makes sense. I trust Alice's regression reports! :)

@ Jim or Neil: can you please back this out of Aurora 47, if you agree that it caused those regressions?

Comment 24

[Jim Mathies [:jimm]]

I tried to back this out of aurora and hit a major merge conflict. I think Neil will have to look at this.

Comment 25

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi BSmedberg, could you please help back out this patch from Fx47? I was hoping we would have done it before the merge day but it's still early in the beta cycle and it will be nice to not ship with this in 47.0b2. Thanks!

Comment 26

[Neil Deakin]

https://hg.mozilla.org/mozilla-aurora/rev/1347c189b6da67d2142fb266c82f74272b09139b
Bug 1256952, revert this bug, r=me, a=ritu

Comment 27

[Jim Mathies [:jimm]]

(In reply to Neil Deakin from comment #26)
> https://hg.mozilla.org/mozilla-aurora/rev/
> 1347c189b6da67d2142fb266c82f74272b09139b
> Bug 1256952, revert this bug, r=me, a=ritu

This is in 47 as well which is current beta.

Comment 28

[Joel Maher ( :jmaher ) (UTC -8)]

perf win on aurora with the backout:
https://treeherder.mozilla.org/perf.html#/alerts?id=1087

on tart and tscrollx for windowsxp.

Comment 29

[Neil Deakin]

Attachment: revertdragexit

Approval Request Comment
[Feature/regressing bug #]: This patch reverts this bug as is the case in Aurora.
[User impact if declined]: Drag operations that occur over the toolbar/tabbar don't always work on Linux e10s.
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: none, simple revert of patch
[String/UUID change made/needed]: none

Comment 30

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8748673 [details] [diff] [review]
revertdragexit

Backing out a patch, Beta47+

Comment 31

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Patch backout, reverting status-47 to affected. NI Sheriffs just to be safe.

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/98e63df2b716

Comment 33

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Hrm, should this be restored to 'fixed' or left as 'affected'?

Comment 34

[Benjamin Smedberg]

I am very confused by the state of this bug. This was backed out of 47 and 48 but was not backed out of 49?  Shouldn't we back this out of all channels and REOPEN the bug?

Comment 35

[David Durst [:ddurst]]

Ugh, yes; my oversight.

Neil, can you please back it out of 49 too?

And, Wes, I assume 47 and 48 both set to 'affected'.

Comment 36

[Neil Deakin]

Yes, I can't do this right now. Please back it out everywhere.

Comment 37

[Pulsebot]

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

Comment 38

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8735559 [details] [diff] [review]
Fire a dragexit event when leaving the browser

This was backed out of Aurora, A-

Comment 39

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8748673 [details] [diff] [review]
revertdragexit

This was backed out of beta, A-

Comment 40

[Carsten Book [:Tomcat]]

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

Comment 41

[Liz Henry (:lizzard) (relman/hg->git project)]

This should be set to either affected or wontfix for 49. Marking it as affected for now since the bug is still open.

Comment 42

[Neil Deakin]

Attachment: Fire a dragexit at the content process

This patch is similar but doesn't set the flag that indicates that the child will update the drag state. The issue is that dragexit in the child never does this (only dragover) so gtk never gets an updated drag action.

This fixes this bug and 1265966 and 1266435. I don't know whether it fixes the crash from bug 1263601 as I can't reproduce it.

Comment 43

[Liz Henry (:lizzard) (relman/hg->git project)]

Neil are you going to land this?

Comment 45

[Neil Deakin]

This bug is a P2 tracking-e10s bug. The original patch is backed out. It doesn't need to track any releases at this time.