Closed Bug 746993 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: DOM: Editor, defect)

Product:
Core
Component:
DOM: Editor
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla14
Tracking Flags:
Tracking Status
firefox14 + fixed

People

(Reporter: mconley, Assigned: mconley)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Fix Patch v1
1.74 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review
Test case v2
4.09 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review 
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.
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
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
Blocks: 743632
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
Attached patch Fix Patch v1 — — Splinter Review 

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Test case v1 (obsolete)
        — 
          — Splinter Review
      

    


  

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Test case v2
        — 
          — Splinter Review
      

    


  
Fixing the whitespace in Makefile.in

        Attachment #616984 -
        Attachment is obsolete: true

    
  

        Attachment #616983 -
        Flags: review?(ehsan)

    
  

        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.

    
  
Target Milestone: --- → mozilla14
Status: NEW → RESOLVED
Closed: 12 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?

    
    

    
  
(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

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: