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

REOPENED
Assigned to

Status

()

P2
normal
REOPENED
3 years ago
9 months 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)

Updated

3 years ago
tracking-e10s: --- → m9+

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
Created attachment 8735559 [details] [diff] [review]
Fire a dragexit event when leaving the browser

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

Updated

3 years ago
Attachment #8735559 - Flags: review?(bugs)
tracking-e10s: m9+ → +
Priority: -- → P2

Comment 7

3 years ago
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
status-firefox48: affected → fixed
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)

Updated

3 years ago
status-firefox47: --- → affected
(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
status-firefox48: fixed → 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+
status-firefox46: --- → wontfix

Comment 14

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/86fe93b28b17
status-firefox47: affected → fixed
backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=2334077&repo=mozilla-aurora
status-firefox47: fixed → affected
Flags: needinfo?(enndeakin)
(Assignee)

Comment 16

3 years ago
Created attachment 8740408 [details] [diff] [review]
Aurora version of the fix
Flags: needinfo?(enndeakin)
(Assignee)

Comment 17

3 years ago
Created attachment 8740410 [details] [diff] [review]
Aurora version of the fix
Attachment #8740408 - Attachment is obsolete: true
(Assignee)

Comment 18

3 years ago
Created attachment 8740411 [details] [diff] [review]
Aurora version of the fix

One last time.
Attachment #8740410 - Attachment is obsolete: true

Comment 19

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/6a7001e456f1
status-firefox47: affected → fixed

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)

Comment 24

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

Comment 27

3 years ago
(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
Created attachment 8748673 [details] [diff] [review]
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
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.
status-firefox47: fixed → affected
Flags: needinfo?(wkocher)
Flags: needinfo?(cbook)

Comment 32

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/98e63df2b716
status-firefox47: affected → fixed
Hrm, should this be restored to 'fixed' or left as 'affected'?
Flags: needinfo?(wkocher)
Flags: needinfo?(cbook)

Comment 34

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

Updated

3 years ago
Status: VERIFIED → REOPENED
status-firefox47: fixed → affected
status-firefox48: verified → affected
status-firefox49: --- → affected
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-

Updated

3 years ago
status-firefox47: affected → wontfix
status-firefox48: affected → wontfix
(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
status-firefox49: affected → fixed
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.
status-firefox49: fixed → affected
tracking-firefox49: --- → +
(Assignee)

Comment 42

3 years ago
Created attachment 8752268 [details] [diff] [review]
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.
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.
status-firefox49: affected → wontfix
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] → [fce-active]

Updated

2 years ago
Duplicate of this bug: 1275303

Updated

a year ago
status-firefox58: --- → affected
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.