1399722 - input boxes fail to register typing after contents cleared

Status: RESOLVED FIXED

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

Description

[Harlan Lieberman-Berg]

Attachment: mwe.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170913220121

Steps to reproduce:

When the value of an inputbox is cleared through javascript, the inputbox retains focus visually but no typing is registered.


Actual results:

Unfocusing and refocusing the inputbox will allow typing to be registered in that input box again.


Expected results:

A MWE is attached.

Comment 1

[Harlan Lieberman-Berg]

I bisected this problem to the following changeset:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9360c4975d3652abcae684e4af531fae7eb93789&tochange=c579ac37ac11a44b9af0c75720f519a40c439b19

Considering the content of the log, I think it is very probable that this is the commit which introduces the regression.  Adding the blocking.

Comment 2

[Harlan Lieberman-Berg]

For the minimum working example, select the text box, type anything, hit enter, and then try to type again.

Comment 3

[Makoto Kato [:m_kato]]

ehsan, this is regression by bug 1385514.  Could you look this?  If no time, I will look this.

Comment 4

[(no longer active)]

I'll take a look.

Comment 5

[(no longer active)]

Attachment: minimized testcase

Comment 6

[(no longer active)]

I didn't end up having time to look into it today, sorry.  I took half the day off sick.  :/

Comment 7

[Julien Cristau [:jcristau]]

Likely too late for 56 now.

Comment 8

[Andrew Overholt [:overholt]]

Should thsi maybe be a 56 blocker? Maybe back something out instead?

Comment 9

[(no longer active)]

[Tracking Requested - why for this release]:

Yes, this needs to block 56.  The decision in comment 7 was wrong.  We should be able to very safely back out bug 1385514 from 56.

Comment 10

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

OK, sounds like we should do that backout. This may affect many sites.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Fixed for Fx56 by backing out bug 1385514.

https://hg.mozilla.org/releases/mozilla-beta/rev/678bea28fe03 (FIREFOX_56b13_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/2bb9e6526ce9

Comment 12

[(no longer active)]

The cause of the bug is that this branch ends up not being taken after bug 1385514:

https://searchfox.org/mozilla-central/rev/1c13d5cf85f904afb8976c02a80daa252b893fca/editor/libeditor/EditorBase.cpp#2747

SetTextImpl() only gets called by WillSetText(), so we can fix this either by backing out the original change or by removing that condition.  But I think the right thing to do is to do both actually, since there is no actual transaction any more, so it doesn't make much sense for SetTextImpl() to check whether subtransactions are supposed to modify the selection in the first place.

Comment 13

[(no longer active)]

Attachment: Don't prevent changing the selection in the editor when setting the value attribute

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8909597 [details] [diff] [review]
Don't prevent changing the selection in the editor when setting the value attribute

>diff --git a/editor/libeditor/tests/test_bug1399722.html b/editor/libeditor/tests/test_bug1399722.html
>+<script class="testbody" type="application/javascript">
>+SimpleTest.waitForExplicitFinish();
>+SimpleTest.waitForFocus(function() {
>+  let elm = document.querySelector("input");
>+
>+  elm.focus();
>+  synthesizeKey("A", {});
>+  synthesizeKey("B", {});
>+  synthesizeKey("VK_RETURN", {});

nit: Could you use "KEY_Enter" instead of legacy "VK_*"?

Comment 15

[(no longer active)]

I need to land this with the patch I posted in bug 1401225 which is pending review.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4193c11e97ae
Don't prevent changing the selection in the editor when setting the value attribute; r=masayuki

Comment 17

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

https://hg.mozilla.org/mozilla-central/rev/4193c11e97ae