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

REOPENED
Assigned to

Status

()

defect
P2
normal
REOPENED
3 years ago
a year ago

People

(Reporter: arni2033, Assigned: enndeakin)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox46 wontfix, firefox47 wontfix, firefox48 wontfix, firefox49+ wontfix, firefox58 affected)

Details

(Whiteboard: [fce-active-legacy])

Attachments

(3 attachments, 4 obsolete attachments)

Reporter

Description

3 years ago
>>>   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 hidden (spam)

Comment 2

3 years ago
Neil, could you take this?
Assignee: nobody → enndeakin
Flags: needinfo?(enndeakin)
Assignee

Comment 3

3 years ago
I suspect this caused by the same issue as 1256162, which I will have a fix for soon.
Flags: needinfo?(enndeakin)

Updated

3 years ago
Depends on: 1256162
Assignee

Comment 4

3 years ago
OK I could never reproduce this but can we check if 1256162 fixed this?
Flags: needinfo?(arni2033)
Reporter

Comment 5

3 years ago
Scenario (C) is not fixed.
Flags: needinfo?(arni2033)
Assignee

Comment 6

3 years ago
A dragexit isn't being fired when moving outside the browser frame.
Assignee

Updated

3 years ago
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+
Assignee

Comment 8

3 years ago
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

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/aa65acd9650a
Status: NEW → RESOLVED
Last Resolved: 3 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)
Reporter

Comment 12

3 years ago
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.
Status: RESOLVED → VERIFIED
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+
Assignee

Comment 16

3 years ago
Posted patch Aurora version of the fix (obsolete) — Splinter Review
Flags: needinfo?(enndeakin)
Assignee

Comment 17

3 years ago
Posted patch Aurora version of the fix (obsolete) — Splinter Review
Attachment #8740408 - Attachment is obsolete: true
Assignee

Comment 18

3 years ago
Posted patch Aurora version of the fix (obsolete) — Splinter Review
One last time.
Attachment #8740410 - Attachment is obsolete: true

Updated

3 years ago
Depends on: 1265966

Updated

3 years ago
Depends on: 1266435

Comment 20

3 years ago
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)
Assignee

Comment 21

3 years ago
(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.
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)

Updated

3 years ago
Flags: needinfo?(benjamin) → needinfo?(ddurst)
(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.
Assignee

Updated

3 years ago
Flags: needinfo?(enndeakin)
perf win on aurora with the backout:
https://treeherder.mozilla.org/perf.html#/alerts?id=1087

on tart and tscrollx for windowsxp.
Assignee

Comment 29

3 years ago
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]
revertdragexit

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)
Assignee

Comment 36

3 years ago
Yes, I can't do this right now. Please back it out everywhere.
Flags: needinfo?(enndeakin)
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
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]
revertdragexit

This was backed out of beta, A-
Attachment #8748673 - Flags: approval-mozilla-beta+ → approval-mozilla-beta-
Assignee

Updated

3 years ago
Blocks: 1263601

Comment 40

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cbe57261cb0a
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED

Updated

3 years ago
Status: RESOLVED → REOPENED
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.
Assignee

Comment 42

3 years ago
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)
Duplicate of this bug: 1265966
Assignee

Comment 45

3 years ago
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)
Assignee

Updated

3 years ago
Attachment #8735559 - Attachment is obsolete: true
Assignee

Updated

3 years ago
Attachment #8740411 - Attachment is obsolete: true
Whiteboard: [fct-active]
Whiteboard: [fct-active] → [fce-active]
Duplicate of this bug: 1275303

Updated

2 years ago
Target Milestone: mozilla48 → ---
Whiteboard: [fce-active] → [fce-active-legacy]
You need to log in before you can comment on or make changes to this bug.