Can't join selected cells in same column in table

VERIFIED FIXED in mozilla0.9.3



17 years ago
16 years ago


(Reporter: rohan.hart, Assigned: Charles Manske)




Firefox Tracking Flags

(Not tracked)


(Whiteboard: fixed, nsBranch+, PDT+)


(1 attachment)



17 years ago
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

17 years ago
confirming; I see this on Macintosh so it isn't an SGI/Irix specific bug.
Reassign this to myself for now; cc cmanske
Assignee: beppe → brade
Ever confirmed: true
Keywords: regression
OS: IRIX → All
Hardware: SGI → All
Summary: Can't join selected cells in table → Can't join selected cells in same column in table


17 years ago
Target Milestone: --- → mozilla0.9.3


17 years ago
Assignee: brade → cmanske

Comment 2

17 years ago
this should go to cmanske, reassigning


17 years ago
Priority: -- → P2

Comment 3

17 years ago
joining should work in column and row selection

reviewed and approved
Keywords: nsBranch

Comment 4

17 years ago
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

17 years ago
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

17 years ago
Created attachment 41298 [details] [diff] [review]
Simplify nsHTMLEditor::GetFirstSelectedCellInTable to fix the bug

Comment 7

17 years ago
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:

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();

  // 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;

Keywords: patch, review
Whiteboard: FIX IN HAND, need r=, sr=, a=

Comment 8

17 years ago

Comment 9

17 years ago

Comment 10

17 years ago
Checked in.
Keywords: patch, review → vtrunk
Whiteboard: FIX IN HAND, need r=, sr=, a= → FIX IN HAND

Comment 11

17 years ago
tested on 7/6 trunk build -- works great
Whiteboard: FIX IN HAND → fixed, nsBranch+

Comment 12

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

Comment 13

17 years ago
per conversation with selmer, adding PDT+ and removed vtrunk
Keywords: vtrunk
Whiteboard: fixed, nsBranch+ → fixed, nsBranch+, PDT+

Comment 14

17 years ago
Checked into trunk and branch.
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 15

17 years ago
verified in 7/10 branch build.

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

Comment 16

17 years ago
verified in branch and trunk builds 7/10.
You need to log in before you can comment on or make changes to this bug.