97625 - drag&drop of partial tables can produce invalid html

Status: RESOLVED FIXED

(SeaMonkey :: Composer, defect)

Description

[TucsonTester2]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.3+) Gecko/20010828
Netscape6/6.1b1
BuildID:    20010828

If a table has no defined width and is dragged into another table the table is
copied.

Reproducible: Sometimes
Steps to Reproduce:
1.Open Composer
2.Insert a 2x2 table and delete the width and leave it blank
3.Put some text in each cell
4.Click out of the table and place the cursor on the line below
5.Hit enter one time and insert another 2x2 table and delete the
width6.Highlight the first table you made and drag it into the left cell in the
second row

Actual Results:  Table was dragged into the cell but it was a copy of the table.
 The first copy was still above the table where it was created.

Expected Results:  I expect that the table would not be copied into the table. 
Instead the table should be moved into to the table.

Comment 1

[TucsonTester2]

This is reproducible everytime.

When I dragged a table into another table it was copied instead of moved.  The
table was only copied if I dragged it into a cell just outside of the table
border.  This only works if it is dragged to the left or the right sides of the
borders, it does not work above or below them.

Steps to Reproduce:
1.Open Composer
2.Insert a default 2x2 100% width table using the table button on the toolbar
3.Insert a table below the first one
4.Right Click the first table and go to select and choose table
5.Drag the table into the top left cell of the second table.  Make sure the
pointer is just outside of the left table border.
6.Release the mouse button

You will notice that the table was copied and not moved.  If you look at the
html source it will actually show the table was copied as well.

What I expect to happen is that the table will be moved and not copied,
regardless of where I have the arrow.  If it supposed to be moved above or below
the table then that should happen as well, instead of the copying problem.

Comment 2

[sujay]

--> charley

Comment 3

[Kathleen :Brade]

-->brade (d&d)

Comment 4

[Syd Logan]

spam composer change

Comment 5

[Kathleen :Brade]

I don't see any problems if I select the whole table and then drag/drop.

The problem I do see is that if the selection starts in a text node inside a 
table, we delete the text nodes but never any of the table structure.  I expect 
to see wholy contained <td> to be deleted.

The other (more serious) problem I see is (possibly in nsHTMLDataTransfer.cpp?) 
where we insert <tr> directly into a <td>.  This asserts a few times and 
eventually creates a document with invalid html.  Also, this can't be undone.
Note: if there is a character in the table cell where you drop, you won't see the 
problem (a table gets inserted properly and undo works).

Reassign to jfrancis for input/fix.

Comment 6

[Joe Francis]

Kathy, I didn't write any of the d&d code at either end, so I could use some 
advice.  I know that the drop code does not do the things I do in the paste code.  
That's what causes the drop to generate illegal html.  But I don't even know 
where the drag code lives.  Where is the code that should drive the deletion of 
the table in the example above?  Does anyone still own it?  If I describe the 
kind of things that need to happen there is there anyone around to do it?

assigning to brade for the moment to make sure she see's my questions.

Comment 7

[Kathleen :Brade]

changing summary to accurately reflect severity of this bug (please file other
issues as new bugs if you can still reproduce them; sorry for any inconvenience)

Drag&Drop code is in nsHTMLDataTransfer.cpp.  In particular, InsertFromDrop
should handle the deletion of the table with DeleteSelection() call (but maybe
the variables aren't set up correctly if the table is selected (does selection
appear collapsed if a table is selected?)

At the end of InsertFromDrop, you'll see that InsertFromTransferable is called.
 At this point, drop and paste should be the same:
  InsertFromTransferable calls InsertHTMLWithContext() for html flavors.
  InsertHTMLWithContext calls InsertHTMLWithCharsetAndContext()...

Let me know if you want me to investigate this a bit more (reassign back to me).
-->jfrancis

Comment 8

[Joe Francis]

pushing off 098 to 099

Comment 9

[Syd Logan]

minusing, doesn't seem common usage to us.

Comment 10

[Joe Francis]

setting milestones based on reprioritization of buglist

Comment 11

[Joe Francis]

The trunk is the wave of the future!

Comment 12

[Joe Francis]

The days of having a half dozen milestones out in front of us to divide bugs 
between seem to be gone, though I dont know why.  Lumping everything together as 
far out as I can.  I'll pull back things that I am working on as I go.

Comment 13

[Joe Francis]

migrating milestones now that there are more available.  i'll pull these back as
I work on them

Comment 14

[saari (gone)]

EDITORBASE+ topembed+ normalization

Comment 15

[Joe Francis]

This is fixed by patch in 183582.

Comment 16

[Kevin McCluskey (gone)]

reassign to [e-mail] since it would be fixed by bug 183582

Comment 17

[Kai Engert [:KaiE:]]

Marking fixed, because bug 183582 got fixed.
Please reopen if you still see the problem.