1415416 - Problem with cursor, it doesn't appear on focused textbox

Status: VERIFIED FIXED

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

Description

[Alex]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.89 Safari/537.36

Steps to reproduce:

When i change readOnly and value in texbox and press tab key the cursor does not appear on focused textbox





Actual results:

Ex:
https://jsfiddle.net/b2y7horx/
When focus on text1 and press tabkey, it focus on text2 but the cursor does not show


Expected results:

The cursor appear on textbox

Thank you FF team

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

I didn't see this in 55, seems a regression after that somewhere.

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

This is the regression-window https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9af23c413a1f8d337b19b4f8450e241e91b71136&tochange=d2aff6fc075dc4a0e5ca5b4cf7d530979942d4b4

I failed to determine a specific commit which may cause this. Could you take a look at the window and see if you have thoughts about which seems suspicious, Jessica?

Comment 3

[Alice0775 White]

Regressed by: 	c46231bce5bc	Makoto Kato — Bug 1375910 - Don't remove text node when setting value is empty string. r=masayuki

:m_kato
Your patch seems cause the regression. Can you look into this?

Comment 4

[Makoto Kato [:m_kato]]

It seems to call collapse in this case.  But it might cause performance regression.

Comment 5

[Makoto Kato [:m_kato]]

nsTextControlFrame::SelectAllOrCollapseToEndOfText won't detect that text is empty.  So we should add logic to detect empty correctly or use nsIEditor's method.

Comment 6

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d070d3e14e2833191270705fc55e264ef5e2cd7

Comment 7

[Makoto Kato [:m_kato]]

Attachment: Bug 1415416 - Part 1. Collapse selection on focus when value is empty.

After landing bug 1375910, input.value = "" doesn't remove inter text node.
When value is empty, SelectAllOrCollapseToEndOfText will set selection to
end of text.  But since empty text node is still exist, it select all nodes,
instead of collapsed.  So if text node is empty, we should collapse selection.

Review commit: https://reviewboard.mozilla.org/r/212020/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212020/

Comment 8

[Makoto Kato [:m_kato]]

Attachment: Bug 1415416 - Part 2. Add test.

Review commit: https://reviewboard.mozilla.org/r/212022/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212022/

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8941772 [details]
Bug 1415416 - Part 1. Collapse selection on focus when value is empty.

https://reviewboard.mozilla.org/r/212020/#review218078

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8941773 [details]
Bug 1415416 - Part 2. Add test.

https://reviewboard.mozilla.org/r/212022/#review218080

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e77e0779441a
Part 1. Collapse selection on focus when value is empty. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/02e365e09e79
Part 2. Add test. r=masayuki

Comment 12

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/e77e0779441a
https://hg.mozilla.org/mozilla-central/rev/02e365e09e79

Comment 13

[Julien Cristau [:jcristau]]

Not a new issue, we're at RC week => wontfix for 58.

Comment 14

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 58.0a1 (2017.11.07) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 59.0b5 on Win 8.1 x64 and Mac OS X 10.13.2 and Ubuntu 14.04 x64.