Joining table cells in the editor is horked

RESOLVED FIXED in Firefox 44

Status

()

Core
Editor
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: glazou, Assigned: glazou)

Tracking

(Blocks: 1 bug, {regression})

Trunk
mozilla44
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Steps to reproduce:

1. use for instance last nightly of Seamonkey
2. insert a 2x2 table into the document
3. select two adjacent table cells
4. use context menu to join the cells

EXPECTED RESULT: cells are joined, resulting cell is selected
ACTUAL RESULT: nothing happens, nsITableEditor.joinTableCells() throws an NS_ERROR_FAILURE
(Assignee)

Updated

5 years ago
Keywords: regression, regressionwindow-wanted
(Assignee)

Comment 2

5 years ago
Seamonkey rev 8a30e07815ff 14-dec-2012 works fine.
Seamonkey rev 18ff01530d1d 15-dec-2012 is horked.

glazou$ hg log --rev 8a30e07815ff:18ff01530d1d editor/libeditor/html/nsTableEditor.cpp 
changeset:   116042:3d898c39d05e
user:        Trevor Saunders <trev.saunders@gmail.com>
date:        Wed Aug 08 09:05:17 2012 -0400
summary:     bug 781409 - remove nsITableLayout r=roc,davidb

This is very likely the regression source. The fix for that bug got rid
of NS_EDITOR_ELEMENT_NOT_FOUND in nsHTMLEditor::GetCellDataAt() and that's
almost certainly the issue...
(Assignee)

Comment 3

5 years ago
Created attachment 781654 [details] [diff] [review]
898321.patch

Absolutely trivial fix. Successfully tested in BlueGriffon:-)
Attachment #781654 - Flags: superreview?(neil)
Attachment #781654 - Flags: review?(ehsan)
(Assignee)

Updated

5 years ago
Keywords: regressionwindow-wanted

Updated

5 years ago
Attachment #781654 - Flags: superreview?(neil) → superreview+

Updated

5 years ago
Blocks: 781409

Comment 4

5 years ago
Comment on attachment 781654 [details] [diff] [review]
898321.patch

Actually I had failed to notice that NS_EDITOR_ELEMENT_NOT_FOUND is a success code, so technically you need to clear all your outparams too. (The change appears to work because all of the callers use an nsCOMPtr for the cell, which is still its default of nullptr when they come to check.)
Attachment #781654 - Flags: superreview+

Comment 5

5 years ago
Comment on attachment 781654 [details] [diff] [review]
898321.patch

Review of attachment 781654 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with Neil's comment addressed.  Also, a test case would be really nice here, to make sure that we don't break it in the future.
Attachment #781654 - Flags: review?(ehsan) → review+

Comment 6

5 years ago
I am still on SeaMonkey version 2.16.1, as this was the last version in which the command 'join selected cells' still worked. Any idea when this issue might be fixed ?

BTW, as I don't understand the above techno-speak I am not sure whether it is supposed to contain a remedy ...

wefalck

Comment 7

5 years ago
Daniel, is this patch ready to be checked in?  Have you posted it to the try server?
Flags: needinfo?(daniel)

Updated

5 years ago
Assignee: nobody → daniel

Comment 8

4 years ago
came to this bug via 962557.  Can someone confirm that it is only a merge involving the last row of the table that is affected.  Bug 962557 has no entry in the error console and merge only fails if the last row of the table is included in the merge.

Updated

4 years ago
Blocks: 962557

Comment 9

3 years ago
Can someone drive this patch in? It doesn't look like Daniel is active on this any more...
This will hopefully fix bug 962557 in the process.

Comment 10

2 years ago
The trivial patch is and the "needinfo" are over two years-old! Let's see, if clearing it will get the attention of some _active_ developer...
Flags: needinfo?(daniel)

Comment 11

2 years ago
Created attachment 8659873 [details] [diff] [review]
Updated version of the patch for comm-esr38 tree

Apparently, the libeditor-tree has been reorganized somewhat -- this version of the patch changes the path to the modified file.

I just rebuilt Thunderbird 38.2.0 with it and the cell-joining now works fine for me.

Comment 12

2 years ago
I have just downloaded Seamonkey 2.35 for Mac and the joining of cells still does not work. I am on MacOS 10.7.5.
Try run for patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e05ec34dc3af

I will check in the patch if it passes try.

Does anyone have a way to reproduce this in Firefox so I can write a test for Gecko?  Is this interface exposed to websites in any way?

(In reply to neil@parkwaycc.co.uk from comment #4)
> Actually I had failed to notice that NS_EDITOR_ELEMENT_NOT_FOUND is a
> success code, so technically you need to clear all your outparams too. (The
> change appears to work because all of the callers use an nsCOMPtr for the
> cell, which is still its default of nullptr when they come to check.)

This doesn't seem to be relevant, because the only outparam is aCell, which is already cleared, unless I'm missing something.  Do we really need to re-clear this?  I will anyway in the patch to avoid getting new review, just to be safe.
https://hg.mozilla.org/mozilla-central/rev/fd48d3450594
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.