1237286 - In some cases <textarea> displays caret on wrong line, and when I start typing, caret teleports to the next line (caret out of sync with collapsed selection) [2/2]

Status: REOPENED

(Core :: DOM: Selection, defect, P5)

Description

[arni2033]

Attachment: testcase 1 - textarea displays cursor on wrong line.html

>>>   My Info:   Win7_64, Nightly 46, 32bit, ID 20160105030211
STR:
1. Open attached "testcase 1"
2. Click at the end of the last line (you'll get "asdf[cursor]")
3. Press Shift+Up to select text "\nasdf"
4. Press (Shift+)Enter to paste "\n" instead of selected text
5. Type "x"

Result:       
 After Step 4 cursor is displayed at the end of the 1st line
 After Step 5 the typed letter ("x") appears in the beginning of the 2nd line

Expectations (either A or B): 
 A) After Step 4 cursor should be displayed at the start of the 2nd line
 B) After Step 5 the typed letter ("x") should appear in the end of the 1st line

Regression range:
>   https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e9769c80ee593c9981cb00079801ae040ba55213&tochange=02f2f4c75007651c63bbc0791d9a58dea88f545f

I detected this and bug 1237236 about 4 month ago and thought they were the same bug
I will appreciate it if somebody will improve the summaries of this bug and bug 1237236

Comment 1

[arni2033]

Attachment: screencast 1 - textarea displays cursor on wrong line.webm

Comment 2

[Jorg K (CEST = GMT+2)]

Not sure why this should block bug 756984. That bug was fixed and landed on Gecko 39.
You're saying that this was regressed by bug 756984, right?

In that bug we changed the selection behaviour when <br> elements are used. The selection is placed before the brFrame when positioning to the end of the line either by clicking to the right of the end of the line, using the "end" key or moving up using the "right" key from the start of the following line.

Your example contains a textarea and there aren't any <br> elements.

Bug 1237236 really looks very similar. Here and there you have textareas and you create a selection that includes the line break.

Comment 3

[arni2033]

> You're saying that this was regressed by bug 756984
> Your example contains a textarea and there aren't any <br> elements.
1) Yes, I thought it was...
2) There's only anonymous <br> element (see screenshot below). If you're totally sure that 756984
   couldn't cause this bug, then I'll probably set keyword "regressionwindow-wanted".
> screenshot: https://dl.dropboxusercontent.com/s/jp8bur2z0rnz3y4/bug%201237286%20comments%201.png

Comment 4

[Jorg K (CEST = GMT+2)]

In the DOM inspector the text content shows as "A\nasdf", so there really is no <br> in the textarea.
I can only be totally sure if I back out the change from bug 756984.
Sometimes a regression window doesn't help, since the bug that caused the problem cannot be backed out or sometimes a change exposes another bug. Without looking at it in detail, I don't know. Once again, the other bug 1237236 looks very similar, so I'd start there. By the looks of it the caret position doesn't accurately reflect the selection, so a subsequent insertion goes somewhere unexpected.

Comment 5

[arni2033]

> there really is no <br> in the textarea 
It's not just screenshot, there IS <br> in textarea. You can inspect anonymous nodes by setting pref
devtools.inspector.showAllAnonymousContent -> true   and then restarting the browser
(I only left this comment to fix the wrong statement)

Comment 6

[Jorg K (CEST = GMT+2)]

Attachment: DOM of textarea

Sorry, I don't see the <br>

Comment 7

[arni2033]

As I told: set that pref to true, restart browser in safe mode, and use normal Inspector to inspect
the textarea. "DOM inspector" and other tools provided by extensions aren't always reliable.

Comment 8

[arni2033]

Marking this as dependant bug based on comment 4 (since I agree with everything except <br> thing)

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: Backing out part of bug 756984 to see what difference it makes.

Backing out a bit of bug 756984 (https://hg.mozilla.org/mozilla-central/rev/bf414f68291c#l3.19) makes this problem go away. So it is a regression from bug 756984 after all.

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: DOM of textarea (take 2)

I stand corrected, there is a <br> after the visible text ;-)

Looks like the DOM is this:
A\nasdf<br>

Before bug 756984 landed, going to the end of the line put the selection behind the <br> as indicated with "|":
A\nasdf<br>|

With bug 756984 landed, we now place the selection before the <br>, and this is desired and not a bug:
A\nasdf|<br>

As I said in comment #4:
... the bug that caused the problem cannot be backed out [and the] change exposes another bug

This is what has happened here: The behaviour of the system changed ever so slightly and exposed another bug.

Since the exposed other bug looks very similar to what was reported in bug 1237236, there is hope that both can be fixed together.

Comment 11

[Jorg K (CEST = GMT+2)]

Just looking at this again. The testcase that shows the error is this:

<textarea style="height:200px;">
A
asdf

If you use complete HTML, so

<textarea style="height:200px;">
A
asdf
</textarea>

the problem doesn't occur.

Comment 12

[Jorg K (CEST = GMT+2)]

I have to correct what I said in comment #11.

Not working:
<textarea style="height:200px;">
A
asdf</textarea>

Working:
<textarea style="height:200px;">
A
asdf
</textarea>

So it's the extra newline that makes the difference.

Comment 13

[Jorg K (CEST = GMT+2)]

I debugged this a little bit:
At https://dxr.mozilla.org/mozilla-central/source/editor/libeditor/nsPlaintextEditor.cpp#798
Collapse() is called with offset == 2, after inserting a \n a few lines above. That looks correct to me, since the caret should be placed after the A and the \n in the text area.

I don't understand why visually it's not placed there. I traced through the two cases described in the preceding comment (comment #12) and can't see a difference in processing, yet the outcome is different.

Since you had no trouble fixing the "sister" bug 1237236, maybe you can shed some light on this, too.

Oh, another observation: The STR say, place the cursor behind the asdf, press shift <up>, then hit enter. So this is a "backward" selection. If you place the cursor behind the A and then select to the end of the asdf, which should result in the very same selection containing the \nasdf but this time selecting "forward", and then hit enter, there is no problem.

<off topic>
Frankly, I don't see why I was assigned to this bug without asking me first. It is true that bug 756984 - which was implemented with Ehsan's and Robert's guidance - changed the selection behaviour (we collapse at the end of a text node and not after a br when selecting at the end of a br-terminated line), but that doesn't make me responsible for all other bugs that change uncovered. I'm happy to help though ;-)
</off topic>

Comment 14

[Mats Palmgren (inactive)]

I can't reproduce this in a Linux64 Nightly.

I've just landed bug 1237236 on mozilla-inbound.  It's probably worth waiting
for those builds to check if it fixes this bug too.

Comment 15

[Mats Palmgren (inactive)]

(In reply to Jorg K (GMT+1) from comment #13)
> <off topic>
> Frankly, I don't see why I was assigned to this bug without asking me first.

I totally agree.  Assigning bugs to people without asking them first is rude.
needinfo? is the mechanism to use in a situation like this (and the same
goes for bug 1237236 for that matter).

Comment 16

[Jorg K (CEST = GMT+2)]

I've applied your patch from bug 1237236 locally and it doesn't fix this problem as I had hoped. Otherwise I wouldn't have contacted you ;-)

I'm surprised you can't reproduce this. At least on Windows it's easily reproducible with attachment 8704650 [details] following the STR from comment #0.

I've just tried FF 45 on Linux (64bit) and it's reproducible there. I don't have a current FF dev build on Linux.

Comment 17

[Jorg K (CEST = GMT+2)]

(In reply to Mats Palmgren (:mats) from comment #15)
> I totally agree.  Assigning bugs to people without asking them first is rude.
> needinfo? is the mechanism to use in a situation like this (and the same
> goes for bug 1237236 for that matter).
I'm happy to help, but my analysis has taken me nowhere so far. (I'm really a Thunderbird developer. We sometimes visit M-C territory to fix bugs that affect us, mainly in Core::Editor.)

Comment 18

[Jorg K (CEST = GMT+2)]

(In reply to Mats Palmgren (:mats) from comment #14)
> I can't reproduce this in a Linux64 Nightly.

(In reply to Jorg K (GMT+1) from comment #16)
> I've just tried FF 45 on Linux (64bit) and it's reproducible there. I don't
> have a current FF dev build on Linux.
Now I do. I've just refreshed my build on Linux Mint (64bit) just after your bug 1237236 landed.

I *can* reproduce the problem. Can you please take another look.

Comment 19

[Mats Palmgren (inactive)]

I believe you.  I just don't have time to work on this.

Comment 20

[Brad Lassey [:blassey] (use needinfo?)]

Jet, can you help find an assignee?

Comment 21

[Jet Villegas (inactive)]

(In reply to Jorg K (GMT+1) from comment #11)
> Just looking at this again. The testcase that shows the error is this:
> 
> <textarea style="height:200px;">
> A
> asdf
> 
> If you use complete HTML, so
> 
> <textarea style="height:200px;">
> A
> asdf
> </textarea>
> 
> the problem doesn't occur.
>
>Severity: normal → minor

I agree with the "minor" severity noted here. With the workaround suggested, plus the regression risk, I think we can defer fixing this.

Comment 22

[Jorg K (CEST = GMT+2)]

I'm still hoping that this might get fixed by bug 1258308.

Comment 23

[arni2033]

(In reply to Jet Villegas (:jet) from comment #21)
> With the workaround suggested,
What workaround? I constantly get this issue while editing text.

> plus the regression risk
Please show a bug that doesn't have regression risk. Did bug 756984 NOT have that risk?
Don't all bugs about implementing brand new (useless) stuff have regression risk?
Seriously, if your reason is "nobody wants to ever fix this bug" / "this is desired pain for users", then ofc close it again. Other reasons are not convincing.

Comment 24

[Jet Villegas (inactive)]

(In reply to arni2033 from comment #23)
> (In reply to Jet Villegas (:jet) from comment #21)
> > With the workaround suggested,
> What workaround? I constantly get this issue while editing text.

I'm referring to the </textarea> workaround noted in my last comment.

> > plus the regression risk
> Please show a bug that doesn't have regression risk. Did bug 756984 NOT have
> that risk?

<contenteditable> has higher than average regression risk. 

> Don't all bugs about implementing brand new (useless) stuff have regression
> risk?
> Seriously, if your reason is "nobody wants to ever fix this bug" / "this is
> desired pain for users", then ofc close it again. Other reasons are not
> convincing.

I'm not disputing that this is a bug, and that nobody (perhaps you?) will ever fix it. What we want is to explicitly state that we are OK to continue to ship with this regression.

Comment 25

[arni2033]

(In reply to Jet Villegas (:jet) from comment #24)
> I'm referring to the </textarea> workaround noted in my last comment.
That was a workaround for the situation when I load an HTML page I created myself and (for some reason) want to edit text on that page. Anyway there was no worded workaround for all cases.
Of course I know one "workaround": to open Notepad, type desired text there and then paste it in <textarea> in Firefox. Please don't call such inconvenient things a "workaround"

> we are OK to continue to ship with this regression
Not that I was surprised, as I reported ~800 long-standing bugs I encounter every day (and still counting). But "Wontfix" is only for bugs that "will _never_ be fixed" (I've read the instruction)

Comment 26

[Mats Palmgren (inactive)]

(In reply to arni2033 from comment #25)
> But "Wontfix" is only for bugs that "will _never_ be fixed"

Yeah, I agree.  Please keep filing bugs, it's very helpful to have these documented
with simple testcases and a clear STR.  Thanks!

Comment 27

[Jorg K (CEST = GMT+2)]

This will be fixed with bug 1263288. I tested attachment 8740151 [details] [diff] [review] and it fixes this.

Comment 28

[Jorg K (CEST = GMT+2)]

Since bug 1263288 took a different approach, this is not fixed.

Comment 29

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=3 years untouched DOM/Storage bug's priority.

If you have reason to believe this is wrong, please write a comment and ni :jstutte.