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]

REOPENED
Unassigned

Status

()

Core
Selection
--
minor
REOPENED
a year ago
a year ago

People

(Reporter: arni2033, Unassigned)

Tracking

({regression})

Trunk
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
Created attachment 8704650 [details]
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
(Reporter)

Comment 1

a year ago
Created attachment 8704651 [details]
screencast 1 - textarea displays cursor on wrong line.webm
(Reporter)

Updated

a year ago
See Also: → bug 1237236

Comment 2

a year ago
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.
No longer blocks: 756984
(Reporter)

Comment 3

a year ago
> 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

a year ago
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.
(Reporter)

Comment 5

a year ago
> 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 hidden (obsolete)
Comment hidden (obsolete)
(Reporter)

Comment 8

a year ago
Marking this as dependant bug based on comment 4 (since I agree with everything except <br> thing)
Depends on: 1237236

Updated

a year ago
Summary: In some cases <textarea> displays cursor on wrong line, and when I start typing, cursor teleports to the next line [2/2] → 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]

Comment 9

a year ago
Created attachment 8705175 [details] [diff] [review]
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.

Updated

a year ago
Blocks: 756984

Comment 10

a year ago
Created attachment 8705976 [details]
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.
Attachment #8704854 - Attachment is obsolete: true
Assignee: nobody → mozilla
status-firefox45: --- → affected

Comment 11

a year ago
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

Comment 12

a year ago
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

a year ago
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>
Assignee: mozilla → nobody
Component: Layout: Form Controls → Selection
Flags: needinfo?(mats)
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.
Flags: needinfo?(mats)
(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

a year ago
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

a year ago
(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

a year ago
(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.
Flags: needinfo?(mats)
I believe you.  I just don't have time to work on this.
Flags: needinfo?(mats)
(Reporter)

Updated

a year ago
Depends on: 1258308
Jet, can you help find an assignee?
Flags: needinfo?(bugs)
(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.
Flags: needinfo?(bugs)

Updated

a year ago
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → WONTFIX

Comment 22

a year ago
I'm still hoping that this might get fixed by bug 1258308.
(Reporter)

Comment 23

a year ago
(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.
Status: RESOLVED → REOPENED
Flags: needinfo?(bugs)
Resolution: WONTFIX → ---
(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.
Flags: needinfo?(bugs)
(Reporter)

Comment 25

a year ago
(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)
(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!
(Reporter)

Updated

a year ago
Depends on: 1259949

Updated

a year ago
status-firefox45: affected → ---
status-firefox46: affected → ---

Comment 27

a year ago
This will be fixed with bug 1263288. I tested attachment 8740151 [details] [diff] [review] and it fixes this.
Depends on: 1263288

Comment 28

a year ago
Since bug 1263288 took a different approach, this is not fixed.
No longer depends on: 1263288
You need to log in before you can comment on or make changes to this bug.