54253 - Drag and Drop causes data loss

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P1)

Description

[kinmoz]

Startup a blank editor.

Type the following into the editor:

  This is a test

Now select " is a", then click and drag the selection to any point after the 
word "test" and then let go of the mouse button to drop it.

" is a" disappears and is not added after "test" so the content now looks like:

  This test

It should look like:

  this test is a

Comment 1

[kinmoz]

Adding nsbeta3, rtm and dataloss keywords.

Comment 2

[kinmoz]

Forgot to add regression keyword.

Comment 3

[rubydoo123]

This is a regression and when users select drag & drop and drop in the same
window the selected text gets deleted and on drop the data is not dropped.

Comment 4

[Michael La Guardia]

Marking nsbeta3- as I don't believe that this is a stopper for PR3 (undo -
modulo the double undo bug - retrieves the data).  The bug is already nominated
for rtm.

Comment 5

[rubydoo123]

reassigning to brade

Comment 6

[rubydoo123]

drag and drop is a frequently used function, users will expect the function to 
be usable and reliable. Many users are unaware of undo/redo and multiple 
undo/redo.

Kathy, please include the required information per the rtm checkin rules

Comment 7

[anthonyd]

re-assigning to myself.

anthonyd

Comment 8

[Phil Peterson]

PDT agrees [rtm need info] until patch and code reviews are available.

Comment 9

[anthonyd]

Attachment: fix for bug.

Comment 10

[anthonyd]

Attachment: damnit, the real fix for the bug.

Comment 11

[anthonyd]

adding ken and simon for approval and review. please?
thanks,
anthonyd

Comment 12

[Simon Fraser [no longer active]]

This is a bad code pattern:

 if (NS_FAILED(rv) || !newSelectionOffset) return rv?rv:NS_ERROR_FAILURE;

since NS_FAILED(rv) != (rv == 0) (i.e. non-zero rv's can mean success).
Better to write this out in full, or just skip the rv test.

I'd also like to see a description of what the patch is doing.

Comment 13

[anthonyd]

Attachment: fix after kins review w/ relevant comments

Comment 14

[anthonyd]

kin will re-review and give me approval.

anthonyd

Comment 15

[kinmoz]

sr=[e-mail]

Comment 16

[anthonyd]

simon, I changed that checking condition in the latest patch, and I added a 
comment to explain what the patch does into the patch itself.

anthonyd

Comment 17

[Simon Fraser [no longer active]]

r=sfraser

Comment 18

[anthonyd]

pdt team:
got everything I need for this:
sr=kin
r=sfraser
a=kin
patch is attached

anthonyd requesting rtm++

Comment 19

[anthonyd]

sent email topdt team requesting rtm++ status

Comment 20

[Phil Peterson]

rtm++

Comment 21

[anthonyd]

fix checked in to both branch and trunk.
anthonyd

Comment 22

[sujay]

verified in 10/10 build.