Page MenuHomePhabricator

Merging cells to 'delete' entire rows or columns causes errors
Closed, ResolvedPublic8 Estimated Story Points

Description

Environment- beta, test2

1> insert a table.By default it is 4x4.
2> Highlight only the content cells under the first two header and merge them. Do the same for the other two columns. It is now a 2 x 4 table.
3> Select the last header cell and insert after or click on the first header cell and insert before.
JS error “Uncaught TypeError: Cannot read property 'isPlaceholder' of null” appears.

Screenshot attached.

Event Timeline

Swainr created this task.Jan 8 2015, 12:33 PM
Swainr raised the priority of this task from to Needs Triage.
Swainr updated the task description. (Show Details)
Swainr added a project: VisualEditor.
Swainr added a subscriber: Swainr.

Found in production too.

Catrope assigned this task to Esanders.Jan 8 2015, 1:05 PM
Catrope set Security to None.
Krenair added a subscriber: Krenair.
Esanders renamed this task from Ina 2 x 4 table, inserting column before/after gives JS error “Uncaught TypeError: Cannot read property 'isPlaceholder' of null” to In a 2 x 4 table, inserting column before/after gives JS error “Uncaught TypeError: Cannot read property 'isPlaceholder' of null”.Jan 8 2015, 1:29 PM
Esanders renamed this task from In a 2 x 4 table, inserting column before/after gives JS error “Uncaught TypeError: Cannot read property 'isPlaceholder' of null” to Merging cells to 'delete' entire rows or columns causes errors.

The problem is that you are generating an invalid table by merging entire rows. The HTML you generate, with rowspan="3" but not actually spanning any rows will break in most browsers. Our merge tool should be able to deal with this though.

Esanders added a comment.EditedJan 8 2015, 2:13 PM

This is actually conceptually not that straightforward, consider the following merge operation:

This results in table going from 4 logical rows to 3.

Change 183510 had a related patch set uploaded (by Esanders):
Remove redundant rows/cols after cell merge

https://gerrit.wikimedia.org/r/183510

Patch-For-Review

Jdforrester-WMF triaged this task as High priority.Jan 8 2015, 4:25 PM
Jdforrester-WMF lowered the priority of this task from High to Medium.
Jdforrester-WMF moved this task from To Triage to Blocked on the VisualEditor board.
Jdforrester-WMF moved this task from Blocked to Q4 on the VisualEditor board.Jan 10 2015, 1:03 AM

Change 183510 merged by jenkins-bot:
Remove redundant rows/cols after cell merge

https://gerrit.wikimedia.org/r/183510

Checked in beta - no JS errors.

Jdforrester-WMF edited a custom field.Mar 9 2015, 6:11 PM