Bug 9527 - REGRESSION: Pasting text in native text area inserts text one character before it should
Summary: REGRESSION: Pasting text in native text area inserts text one character befor...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Nobody
URL:
Keywords: InRadar, Regression
: 9790 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-06-21 05:00 PDT by David Kilzer (:ddkilzer)
Modified: 2006-07-19 12:54 PDT (History)
6 users (show)

See Also:


Attachments
patch that fixes the bug, no layout tests, no change log, may not be best fix (4.92 KB, patch)
2006-06-25 16:25 PDT, Darin Adler
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2006-06-21 05:00:28 PDT
Steps to reproduce:

1. Open a web page (like this bug!) with a text area.
2. Type in "asdf".
3. Select all and copy "asdf" to the clipboard.
4. Position the cursor between "as" and "df".
5. Hit Cmd-V to paste.

Expected results: "asasdfdf"

Actual results: "aasdfsdf"
Comment 1 Darin Adler 2006-06-25 15:47:58 PDT
The cause of this problem is this code in ReplaceSelectionCommand::doApply:

        if (!isEndOfParagraph(visibleStart) && !isStartOfParagraph(visibleStart)) {
            insertParagraphSeparator();
            setEndingSelection(VisiblePosition(endingSelection().start(), VP_DEFAULT_AFFINITY).previous());
        }

It seems crazy to me that we always insert a paragraph separator for any paste. But given that we try to do so, there are problems with this code.

The main problem is that insertParagraphSeparator is not guaranteed to insert anything. There are two cases where it won't, as you can see by readingInsertParagraphSeparatorCommand::doApply. For example:

    1) If we're in an empty list item, we will break out of the list.
    2) If the startBlock has no parent node, we will do nothing.

In this bug the case is (2). The fix could be as simple as changing the startBlock->parentNode() case to create a new block that's a child of startBlock or to use InsertLineBreakCommand the way the isTableCell case does.
Comment 2 Darin Adler 2006-06-25 16:25:20 PDT
Created attachment 9026 [details]
patch that fixes the bug, no layout tests, no change log, may not be best fix
Comment 3 Justin Garcia 2006-06-26 11:39:43 PDT
(In reply to comment #2)
> Created an attachment (id=9026) [edit]
> patch that fixes the bug, no layout tests, no change log, may not be best fix
> 

I would fix "2) If the startBlock has no parent node, we will do nothing" to fix this.

Also, I think that "1) If we're in an empty list item, we will break out of the list." should only be default behavior for the InsertParagraphSeparatorCommand that results from a keypress.  Add a bool to its constructor.
Comment 4 Alice Liu 2006-07-05 09:11:45 PDT
<rdar://problem/4613519>
Comment 5 David Kilzer (:ddkilzer) 2006-07-08 09:32:11 PDT
*** Bug 9790 has been marked as a duplicate of this bug. ***
Comment 6 Justin Garcia 2006-07-19 12:54:24 PDT
r15528