86009 - Can't join selected cells in same column in table

Status: VERIFIED FIXED

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

Description

[rohan.hart]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; IRIX 6.5 IP32; en-US; rv:0.9.1+) Gecko/20010612
BuildID:    2001061221

Joining selected cells instead joins one cell to the cell on its right. This
worked correctly in build 2001052521

Reproducible: Always
Steps to Reproduce:
1. Select several contiguous cells in a single column
2. bring up context menu or select table menu
3. select "join selected cells"

Actual Results:  One cell merges with that on its right

Expected Results:  Cells will merge with other cells

Comment 1

[Kathleen :Brade]

confirming; I see this on Macintosh so it isn't an SGI/Irix specific bug.
Reassign this to myself for now; cc cmanske

Comment 2

[rubydoo123]

this should go to cmanske, reassigning

Comment 3

[rubydoo123]

joining should work in column and row selection

reviewed and approved

Comment 4

[wkfx2003-bugzilla]

No matter how you select the cell and how many cells you selected,
when "JOIN SELECTED CELL", only the first cell will join
with the cell on its right.
If you select several cell VERTICALLY, the first cell will
join the cell on its right, even though that cell is not selected.

Comment 5

[Charles Manske]

I'm working on this. Beth: Should we nominate for PDT+? Fix looks simple and I
think the cause influences many other table-editing operations.

Comment 6

[Charles Manske]

Attachment: Simplify nsHTMLEditor::GetFirstSelectedCellInTable to fix the bug

Comment 7

[Charles Manske]

Here's what was happening:
The method GetFirstSelectedCellInTable() was an old method written before we
automatically sorted the selected cell ranges as a user selected > 1 cells.
Thus it had logic to find the first selected cell in the table. It turns out
that the "Join Cells" command was the only method that used
"GetFirstSelectedCellInTable()", so we never noticed the
error for any other table editing methods, which used just
"GetFirstSelectedCell()" instead. GetFirstSelectedCellInTable() does the same
thing, but also returns the row and columns indexes of the cell found.
The problem was that GetFirstSelectedCellInTable() called
"GetNextSelectedCell()", which incremented the member variable
mSelectedCellIndex, and then JoinTableCells would also call
"GetNextSelectedCell()" but now mSelectedCellIndex was 1 more than it should
have been, and it wouldn't detect the second selected cell.
The solution to all this is to simplify "GetFirstSelectedCellInTable()" to
get the row / column indexes of the first cell found instead of having to
search through other selected cells; thus we don't need to call
"GetNextSelectedCell()"  at all.
Here's the complete new method, so it's easier to read than the diff:

NS_IMETHODIMP
nsHTMLEditor::GetFirstSelectedCellInTable(nsIDOMElement **aCell,
			PRInt32 *aRowIndex, PRInt32 *aColIndex)
{
  if (!aCell) return NS_ERROR_NULL_POINTER;
  *aCell = nsnull;
  if (aRowIndex)
    *aRowIndex = 0;
  if (aColIndex)
    *aColIndex = 0;

  nsCOMPtr<nsIDOMElement> cell;
  nsresult res = GetFirstSelectedCell(getter_AddRefs(cell), nsnull);
  if (NS_FAILED(res)) return res;
  if (!cell) return NS_EDITOR_ELEMENT_NOT_FOUND;

  *aCell = cell.get();
  NS_ADDREF(*aCell);

  // Also return the row and/or column if requested
  if (aRowIndex || aColIndex)
  {
    PRInt32 startRowIndex, startColIndex;
    res = GetCellIndexes(cell, startRowIndex, startColIndex);
    if(NS_FAILED(res)) return res;

    if (aRowIndex)
      *aRowIndex = startRowIndex;

    if (aColIndex)
      *aColIndex = startColIndex;
  }

  return res;
}

Comment 8

[mjudge]

r=mjudge

Comment 9

[Simon Fraser [no longer active]]

sr=sfraser

Comment 10

[Charles Manske]

Checked in.

Comment 11

[rubydoo123]

tested on 7/6 trunk build -- works great

Comment 12

[wkfx2003-bugzilla]

0706 Work in Win2k also.
Status might change to FIXED?

Comment 13

[rubydoo123]

per conversation with selmer, adding PDT+ and removed vtrunk

Comment 14

[Charles Manske]

Checked into trunk and branch.

Comment 15

[sujay]

verified in 7/10 branch build.

will verify oin trunk also. Then I will mark verified-fixed.

Comment 16

[sujay]

verified in branch and trunk builds 7/10.