Open Bug 1256952 Opened 8 years ago Updated 2 years ago

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


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




Tracking Status
e10s + ---
firefox46 --- wontfix
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 + wontfix
firefox58 --- affected


(Reporter: arni2033, Assigned: enndeakin)



(Whiteboard: [fce-active-legacy])


(3 files, 4 obsolete files)

>>>   My Info:   Win7_64, Nightly 48, 32bit, ID 20160313030418
 [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

 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.

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

 This bug is most likely the same as bug 1233612.
Neil, could you take this?
Assignee: nobody → enndeakin
Flags: needinfo?(enndeakin)
I suspect this caused by the same issue as 1256162, which I will have a fix for soon.
Flags: needinfo?(enndeakin)
Depends on: 1256162
OK I could never reproduce this but can we check if 1256162 fixed this?
Flags: needinfo?(arni2033)
Scenario (C) is not fixed.
Flags: needinfo?(arni2033)
A dragexit isn't being fired when moving outside the browser frame.
Attachment #8735559 - Flags: review?(bugs)
Priority: -- → P2
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)
Attachment #8735559 - Flags: review?(bugs) → review+
Bug 1256952, send a dragexit at remote process when leaving the remote frame, r=smaug
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
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
Attachment #8735559 - Flags: approval-mozilla-aurora?
Hello Arni2033, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(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.
Flags: needinfo?(arni2033)
Comment on attachment 8735559 [details] [diff] [review]
Fire a dragexit event when leaving the browser

Fix was verified, Aurora47+
Attachment #8735559 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attached patch Aurora version of the fix (obsolete) — Splinter Review
Flags: needinfo?(enndeakin)
Attached patch Aurora version of the fix (obsolete) — Splinter Review
Attachment #8740408 - Attachment is obsolete: true
Attached patch Aurora version of the fix (obsolete) — Splinter Review
One last time.
Attachment #8740410 - Attachment is obsolete: true
Depends on: 1265966
Depends on: 1266435
This causes ux regression problem.
I think this should be bucked out from aurora47 before it become beta at least
Flags: needinfo?(rkothari)
Flags: needinfo?(enndeakin)
(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

Flags: needinfo?(enndeakin)
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?
Flags: needinfo?(rkothari)
Flags: needinfo?(jmathies)
Flags: needinfo?(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?
Flags: needinfo?(cpeterson) → needinfo?(enndeakin)
I tried to back this out of aurora and hit a major merge conflict. I think Neil will have to look at this.
Flags: needinfo?(jmathies)
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!
Flags: needinfo?(benjamin)
Flags: needinfo?(benjamin) → needinfo?(ddurst)
(In reply to Neil Deakin from comment #26)
> 1347c189b6da67d2142fb266c82f74272b09139b
> Bug 1256952, revert this bug, r=me, a=ritu

This is in 47 as well which is current beta.
Flags: needinfo?(enndeakin)
perf win on aurora with the backout:

on tart and tscrollx for windowsxp.
Attached patch revertdragexitSplinter Review
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
Attachment #8748673 - Flags: approval-mozilla-beta?
Comment on attachment 8748673 [details] [diff] [review]

Backing out a patch, Beta47+
Attachment #8748673 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Patch backout, reverting status-47 to affected. NI Sheriffs just to be safe.
Flags: needinfo?(wkocher)
Flags: needinfo?(cbook)
Hrm, should this be restored to 'fixed' or left as 'affected'?
Flags: needinfo?(wkocher)
Flags: needinfo?(cbook)
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?
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'.
Flags: needinfo?(ddurst) → needinfo?(enndeakin)
Yes, I can't do this right now. Please back it out everywhere.
Flags: needinfo?(enndeakin)
Comment on attachment 8735559 [details] [diff] [review]
Fire a dragexit event when leaving the browser

This was backed out of Aurora, A-
Attachment #8735559 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora-
Comment on attachment 8748673 [details] [diff] [review]

This was backed out of beta, A-
Attachment #8748673 - Flags: approval-mozilla-beta+ → approval-mozilla-beta-
Blocks: 1263601
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Resolution: FIXED → ---
This should be set to either affected or wontfix for 49. Marking it as affected for now since the bug is still open.
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.
Neil are you going to land this?
Flags: needinfo?(enndeakin)
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.
Flags: needinfo?(enndeakin)
Attachment #8735559 - Attachment is obsolete: true
Attachment #8740411 - Attachment is obsolete: true
Whiteboard: [fct-active]
Whiteboard: [fct-active] → [fce-active]
Target Milestone: mozilla48 → ---
Whiteboard: [fce-active] → [fce-active-legacy]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.