Editor selection caret acts strangely when pressing enter after a <br> node.

RESOLVED FIXED in Firefox 14

Status

()

Core
Editor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

({regression})

Trunk
mozilla14
x86
All
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox14+ fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Just noticed this on nightly.

STR:

1)  Set up an account to reply above the quote
2)  Reply to a message using the HTML composer
3)  Type some characters, press enter, type some characters, press enter

What happens?

The caret doesn't appear to move when pressing enter.  However, the text appears to go onto the next line.

What's expected?

The caret should be placed where the text will be inserted.


I thought this might be fallout from the Filelink insertion stuff I did in nsMsgCompose.cpp, but this problem only appears on Daily, and not EarlyBird.

Currently bisecting.
(Assignee)

Comment 1

5 years ago
Ehsan and I are pretty sure this is an editor bug, since we were able to reproduce this in Firefox nightly.
Component: Message Compose Window → Editor
Product: Thunderbird → Core
QA Contact: message-compose → editor
(Assignee)

Comment 2

5 years ago
STR on Firefox:

1)  Go to http://www-archive.mozilla.org/editor/midasdemo/
2)  Click on "View HTML source"
3)  Insert the following HTML:

<html xmlns="http://www.w3.org/1999/xhtml"><head><meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1" /></head><body text="#000000" bgcolor="#FFFFFF"><br /><div class="moz-cite-prefix">On 19/04/2012 9:07 AM, Mike Conley wrote:<br /></div><blockquote type="cite" cite="mid:4F900E01.1030101@mozilla.com">This is a buggy situation.
<br />
<br />

</blockquote><br /><br /></body></html>

4)  Uncheck "View HTML source"
5)  Type some text at the beginning of the text box, press enter, more typing, press enter.

Problem is as described in https://bugzilla.mozilla.org/show_bug.cgi?id=746993#c0
(Assignee)

Comment 3

5 years ago
mozregression gave me this range:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7702bca6b64d&tochange=e636439e342f
(Assignee)

Updated

5 years ago
Blocks: 743632
(Assignee)

Comment 4

5 years ago
Ehsan helped me debug this, and I've got a patch that seems to fix the issue.

Patch + test coming up next.
Summary: Composer selection caret acts strangely in HTML replies → Editor selection caret acts strangely when pressing enter after a <br> node.
Assignee: nobody → mconley
tracking-firefox14: --- → +
Keywords: regression
(Assignee)

Comment 5

5 years ago
Created attachment 616983 [details] [diff] [review]
Fix Patch v1
(Assignee)

Comment 6

5 years ago
Created attachment 616984 [details] [diff] [review]
Test case v1
(Assignee)

Comment 7

5 years ago
Try build is baking on https://tbpl.mozilla.org/?tree=Try&rev=95206221a65a
(Assignee)

Comment 8

5 years ago
Created attachment 616985 [details] [diff] [review]
Test case v2

Fixing the whitespace in Makefile.in
Attachment #616984 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #616983 - Flags: review?(ehsan)
(Assignee)

Updated

5 years ago
Attachment #616985 - Flags: review?(ehsan)
Comment on attachment 616983 [details] [diff] [review]
Fix Patch v1

Review of attachment 616983 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/libeditor/html/nsHTMLEditRules.cpp
@@ +7683,5 @@
>    PRInt32 selOffset;
>    res = mHTMLEditor->GetStartNodeAndOffset(aSelection, getter_AddRefs(selNode), &selOffset);
>    NS_ENSURE_SUCCESS(res, res);
> +
> +  // are we after a <br>?  If so we want to stick to whatever is after <br>.

Please add a comment as to why we need to do this before checking for block siblings.
Attachment #616983 - Flags: review?(ehsan) → review+
Attachment #616985 - Flags: review?(ehsan) → review+
Please land as a single changeset.  Note that this doesn't need approval.
(Assignee)

Comment 11

5 years ago
Committed to mozilla-inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/7dc2085a8cc6
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/7dc2085a8cc6
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 616985 [details] [diff] [review]
Test case v2

Review of attachment 616985 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/tests/test_reftests_with_caret.html
@@ +130,5 @@
>    tests.push([ 'bug613433-2.html' , 'bug613433-ref.html' ]);   // bug 681332
>    tests.push([ 'bug613433-3.html' , 'bug613433-ref.html' ]);   // bug 681332
>    tests.push([ 'bug613807-1.html' , 'bug613807-1-ref.html' ]); // bug 680574
>    tests.push([ 'bug634406-1.html' , 'bug634406-1-ref.html' ]); // bug 681146
> +  tests.push([ 'bug746993-1.html' , 'bug746993-1-ref.html' ]); // bug 746993

Why doesn't this test pass on Windows, or, if it does, why isn't it run there?
(Assignee)

Comment 14

5 years ago
(In reply to Ms2ger from comment #13)
> Comment on attachment 616985 [details] [diff] [review]
> Test case v2
> 
> Review of attachment 616985 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/base/tests/test_reftests_with_caret.html
> @@ +130,5 @@
> >    tests.push([ 'bug613433-2.html' , 'bug613433-ref.html' ]);   // bug 681332
> >    tests.push([ 'bug613433-3.html' , 'bug613433-ref.html' ]);   // bug 681332
> >    tests.push([ 'bug613807-1.html' , 'bug613807-1-ref.html' ]); // bug 680574
> >    tests.push([ 'bug634406-1.html' , 'bug634406-1-ref.html' ]); // bug 681146
> > +  tests.push([ 'bug746993-1.html' , 'bug746993-1-ref.html' ]); // bug 746993
> 
> Why doesn't this test pass on Windows, or, if it does, why isn't it run
> there?

Just following Ehsan's direction on that one.  Ehsan?
These tests can be very unreliable on Windows, for reasons which are unclear to me as I've never had the time to investigate.  Therefore, every time somebody wants to add a test, I ask them to add them to this section.  Once we determine the root cause of the problem, we can fix them all up wholesale.  This is bug 680164.
Depends on: 680164

Updated

5 years ago
status-firefox14: --- → fixed
You need to log in before you can comment on or make changes to this bug.