Cursor lost after applying colour to text

RESOLVED FIXED in Firefox 52

Status

()

Core
Event Handling
--
major
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: Jorg K (GMT+2), Unassigned)

Tracking

({regression})

52 Branch
mozilla53
regression
Points:
---

Firefox Tracking Flags

(firefox51 unaffected, firefox52+ fixed, firefox53 fixed)

Details

(Reporter)

Description

9 months ago
STR:
Write a new message (open compose window)
type something
select the text
make it red

Result: Cursor is lost.

Not seen in TB 51 beta, see in TB 52 Earlybird and TB 53 trunk.

Seems to be TB specific, since I can't see it in a recent FF (FF 53 of 7th Jan 2017) in the MIDAS demo at:
http://www-archive.mozilla.org/editor/midasdemo/

Alice, this is pretty bad, so please help!
Flags: needinfo?(alice0775)

Comment 1

9 months ago
Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=d4fb23de45465e419feb385b79d1e201dcc82759&tochange=0b4789cf7743bd1e3319eca637b649f9f9491fc9
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=91f5293e9a89056565493ed5073c3842b0ee9fdc&tochange=ac3275723df59db0f09198fdb61b51e7002c391a
Flags: needinfo?(alice0775)
(Reporter)

Comment 2

9 months ago
Hmm, so all this landed on mozilla53. How do you explain that it doesn't work in TB 52 (Aurora)? Did any of this get uplifted to M-A?

And why don't I see it in FF on the Midas demo. (I think) I have a recent trunk build of FF (80eac484366a at Tue Jan 17 13:47:20 2017 -0800) and I can't see it there either.

Comment 3

9 months ago
Regression window(comm-aurora):
https://hg.mozilla.org/releases/comm-aurora/pushloghtml?fromchange=79a97ba8b172398034ca64bf8c195dce3a387aa6&tochange=79a97ba8b172398034ca64bf8c195dce3a387aa6
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=412036d99f78afc1e68c1d7088a3ce1fa275405e&tochange=fd66a1e49411faca500b26ba9ca7dd5a311ff6e5


BTW,
Caret reappears after change active window from compose window to the other window, and then back to compose window.
(Reporter)

Comment 4

9 months ago
Can you see any bug that landed in the M-C range that got uplifted in the M-A range. I can't.

Comment 5

9 months ago
I can also reproduce the caret problem on Nightly53.0a1 if addon ScrapBook X 1.13.1(it can focus change from the editor to dialog).

Steps to reproduce:
1. Open http://www-archive.mozilla.org/editor/midasdemo/
2. type something
3. select the text
4. Right click and click on "Save selection as" (ScrapBook X's context menu) 
5. Cancel on save detail dialog
6. 

Result: Cursor is lost.

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e8b32aaa2eec84c15f44cfd677142a0047481152&tochange=f3ccf0f50160e169389aa26a2d95ec225effcd3b

Suspect: a60d62c55e27	Jessica Jong — Bug 1316330 - Cancel delayed keypress events if last keydown was canceled. r=smaug

Updated

9 months ago
Blocks: 1316330
status-thunderbird52: --- → affected
status-thunderbird53: --- → affected

Comment 6

9 months ago
(In reply to Alice0775 White from comment #5)
> I can also reproduce the caret problem on Nightly53.0a1 if addon ScrapBook X
> 1.13.1(it can focus change from the editor to dialog).
> 
> Steps to reproduce:
> 1. Open http://www-archive.mozilla.org/editor/midasdemo/
> 2. type something
> 3. select the text
> 4. Right click and click on "Save selection as" (ScrapBook X's context menu) 
> 5. Cancel on save detail dialog
> 6. 
> 
> Result: Cursor is lost.
> 
> Regression window:
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=e8b32aaa2eec84c15f44cfd677142a0047481152&tochange=f3cc
> f0f50160e169389aa26a2d95ec225effcd3b
> 
> Suspect: a60d62c55e27	Jessica Jong — Bug 1316330 - Cancel delayed keypress
> events if last keydown was canceled. r=smaug

In addition to the above STR,
To reproduce on Nightly, it should be necessary to disable e10s.
(Reporter)

Comment 7

9 months ago
Jessica and Olli, your bug 1316330 cause a regression. We lose the cursor when editing in Thunderbird, and as Alice outlines in comment #5 this can also be seen in FF alone.

We want to go to beta early next week, so I suggest to back your bug 1316330 out at least from Aurora *now*.
Flags: needinfo?(jjong)
Flags: needinfo?(bugs)

Comment 8

9 months ago
No need any addon.
I can reproduce the caret problem on Nightly53.0a1.
It reproduced with/without e10s.


Steps to reproduce:
1. Open http://www-archive.mozilla.org/editor/midasdemo/
2. type something
3. Click on "Inset link" icon 
5. Cancel on dialog
6. 

Result: Cursor is lost.
status-thunderbird52: affected → ---
status-thunderbird53: affected → ---
Component: Message Compose Window → Event Handling
Product: Thunderbird → Core
Version: Trunk → 52 Branch

Comment 9

9 months ago
[Tracking Requested - why for this release]: affected to web page as well as Thunderbird compose window
status-firefox51: --- → unaffected
status-firefox52: --- → affected
status-firefox53: --- → affected
tracking-firefox52: --- → ?
I found that, when event handling is suppressed, only focus events from a mouse or key are delayed, but all blur events are delayed [1]. So, in this case, when canceling the dialog, focus was fired first (the document was still under EventHandlingSuppressed() but aFocusMethod was false), then the delayed blur event was fired when event handling was unsuppressed. Hence, the final state was blurred.

I need to discuss with Olli how to fix this, so I think it's better to backout bug 1316330 from aurora first. Sorry for the inconvenience.

[1] http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/dom/base/nsFocusManager.cpp#2130
Flags: needinfo?(jjong)
(In reply to Jessica Jong [:jessica] from comment #10)
> I found that, when event handling is suppressed, only focus events from a
> mouse or key are delayed, but all blur events are delayed [1]. So, in this
> case, when canceling the dialog, focus was fired first (the document was
> still under EventHandlingSuppressed() but aFocusMethod was false), then the
> delayed blur event was fired when event handling was unsuppressed. Hence,
> the final state was blurred.
> 
> I need to discuss with Olli how to fix this, so I think it's better to
> backout bug 1316330 from aurora first. Sorry for the inconvenience.
> 
> [1]
> http://searchfox.org/mozilla-central/rev/
> 30fcf167af036aeddf322de44a2fadd370acfd2f/dom/base/nsFocusManager.cpp#2130

It's interesting that with a simpler test case, blur event is fired before the prompt is shown...
I agree, backing out bug 1316330 makes sense.
Flags: needinfo?(bugs)
(Reporter)

Comment 13

9 months ago
Well, it's been backed out from Aurora, if you want to have it backed out from trunk, please take the appropriate steps.
Marking this fixed in 52 and 53 by
https://hg.mozilla.org/mozilla-central/rev/e674ae0954df
https://hg.mozilla.org/releases/mozilla-aurora/rev/4a274293090d
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox52: affected → fixed
status-firefox53: affected → fixed
tracking-firefox52: ? → +
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Jorg, Alice, I uploaded a follow-up patch in bug 1316330 to fix this. I'm wondering are there any other cases, besides from comment 8, that I can test to verify the patch? Thanks.
Flags: needinfo?(jorgk)
Flags: needinfo?(alice0775)
(Reporter)

Comment 16

9 months ago
I've been experiencing the bug in TB, so I'm happy to try your patch in TB. Let's talk in the other bug.
Flags: needinfo?(jorgk)

Comment 17

9 months ago
Try build please.


And I think that an automate test should be provided.
Flags: needinfo?(alice0775)
(In reply to Alice0775 White from comment #17)
> Try build please.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8741a51c924fbeeda851087886bbdfd8efc8b2e6&group_state=expanded

> 
> And I think that an automate test should be provided.

Will add an automation test.

Comment 19

9 months ago
(In reply to Jessica Jong [:jessica] from comment #18)
> (In reply to Alice0775 White from comment #17)
> > Try build please.
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=8741a51c924fbeeda851087886bbdfd8efc8b2e6&group_state=e
> xpanded
> 

I tested with STR comment #8.
With enabled e10s:  It works as expected.
With disabled e10s: I can reproduce the caret problem.

https://hg.mozilla.org/try/rev/8741a51c924fbeeda851087886bbdfd8efc8b2e6
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0 ID:20170124042703
(Reporter)

Comment 20

9 months ago
I can reproduce the problem, too, using the "Insert Link" on Midas. In TB the various "Insert" dialogues don't lose the cursor when applying the two patches from bug 1316330 in a local build.
(In reply to Jorg K (GMT+1) from comment #20)
> I can reproduce the problem, too, using the "Insert Link" on Midas. In TB
> the various "Insert" dialogues don't lose the cursor when applying the two
> patches from bug 1316330 in a local build.

Hmm, where you can reproduce the problem, is it a local build? with e10s or non-e10s? It seems that the fix only applies for e10s.

Thanks.
(Reporter)

Comment 22

9 months ago
I can reproduce the problem in non-e10s with the try build you supplied. I was merely confirming comment #19. STR are to cancel the "Insert Link" on Midas and lose the cursor.
(In reply to Jorg K (GMT+1) from comment #22)
> I can reproduce the problem in non-e10s with the try build you supplied. I
> was merely confirming comment #19. STR are to cancel the "Insert Link" on
> Midas and lose the cursor.

I see, thank you both for help testing.
I'm working on a fix for e10s and non-e10s, will upload a new patch soon.
You need to log in before you can comment on or make changes to this bug.