Closed Bug 489202 Opened 14 years ago Closed 11 years ago
selection moves to top when html containing meta, link, or style elements is pasted or inserted
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:188.8.131.52) Gecko/2009032712 Ubuntu/8.04 (hardy) Firefox/3.0.8 Build Identifier: Certain HTML elements, notably <meta>, <link>, and <style>, cause the cursor position to be lost when pasted into an editable document. This is a common use case since content copied out of Microsoft Word will usually include all three of these elements. This also occurs when the HTML is inserted with document.execCommand('inserthtml', false, htmlContent). Example: 1. Go to midas demo: http://www.mozilla.org/editor/midasdemo/ 2. Copy some formatted text from an MS Word document and paste it into midas demo. Actual result: Cursor jumps to the top of the document. Expected result: Cursor is placed immediately following the pasted content (as it is when pasting plain text). Reproducible: Always
this might be related to bug #493189
I see this problem with nightly Thunderbird build (currently Mozilla/5.0 (X11; Linux x86_64; rv:8.0a1) Gecko/20110716 Thunderbird/8.0a1). Simple Linux test case: * Launch Google Chrome * Browse to a Web site * Open a message compose window in Thunderbird * Type a line of text in the body of the message and hit enter so you're no longer at the top of the message * Copy the URL from the URL field in Chrome * Paste into the compose window with ctrl-v * Note how the cursor jumps to the top of the message
OK, so I've done some research about this, and I think I've fixed the Thunderbird issue I reported in Comment 2, but I'm not longer convinced that the Thunderbird issue is necessarily the same as the issue reported in the bug Description. Or perhaps the bug Description is wrong? Here's what I've discovered... When nsHTMLEditor::DoInsertHTMLWithContext in mozilla/editor/libeditor/html/nsHTMLDataTransfer.cpp is pasting HTML from another document (e.g., you copy from chrome and paste into Thunderbird, rather than copying from one part of the message you're composing and paste into another part), it constructs a document fragment to paste, and this document fragment (obviously) isn't part of the body of the document being pasted into. In this situation, the block of code that actually does the insert is the block of code starting at line 647. This block of code sets lastInsertNode to the document fragment node being inserted ("lastInsertNode = parent"). Later, starting at line 667, the code attempts to set the insertion point to after lastInsertNode. This fails, because lastInsertNode isn't actually in the body of the message being edited -- it's a document fragment node, as noted above. As a result, nsHTMLEditRules::ConfirmSelectionInBody in nsHTMLEditRules.cpp ends up moving the cursor to the beginning of the body, because when selection->Collapse is called at line 741 of nsHTMLDataTransfer.cpp, it's passing in the document fragment node, which isn't in the body, so the failsafe "selection not in body" code in nsHTMLEditRules.cpp kicks in. I confirmed this by putting some debug printf's into ConfirmSelectionInBody and watching it get called with a selection whose tag name is #document-fragment. Attached is my proposed fix to this problem. I can't claim to grok all of this code well enough to know for 100% certain that it is correct, but it seems to work and I don't think it is likely to cause any harm. I worry that there is no one who understands the editor code well enough to be able to confirm my diagnosis and review my fix. :-(
Jonathan, thanks a lot for the detailed analysis and also for the patch. I think your analysis makes a lot of sense, and I think that your patch is correct, but I need two things to be able to r+ this first. 1. Can you tell me how one can reproduce this using Firefox (on www.mozilla.org/editor/midasdemo/ for example)? I wanted to test this but I was not successful in reproducing the problem in Firefox at all. 2. Can you please add a unit test for this? See https://developer.mozilla.org/en/Mochitest for a detailed guide on how unit tests work. You can find a lot of unit test samples in http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/tests/. Note that coming up with a way to reproduce this in Firefox is also going to help with creating a unit test for the bug. Thanks a lot! :-)
Thanks to some lovely help from firstname.lastname@example.org on the #developers IRC channel, I've solved the unit test problem. Attached is a new patch which includes the fix, a unit test, and a patch to the unit test Makefile.in. For the record, Fabien Cazenave also ovvered to help, but Neil beat him to it. Thanks for all the help, folks!
Comment on attachment 548496 [details] [diff] [review] fix, unit test file, unit test Makefile.in patch This looks really really good, Jonathan! Sorry that I didn't catch you on irc (I don't read the channels usually unless someone pings me) so in the future feel free to do so!
Attachment #548496 - Flags: review?(ehsan) → review+
Pushed to mozilla-inbound.
Status: UNCONFIRMED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.