Form containing three tables with "tableDrag" feature for every of them. Every row of table consists of four columns (latest - weight, which is hidden). Also, for every table manually added one extra row, containing a custom button in a single column. For this column, in last row, the colspan attribute is set to 4 to make it as wide as row is.

The logic of tabledrag.js is increasing or decreasing the values of colspan attribute for elements with tabledrag-has-colspan class, depending on initial state (show/hide the weight field), saved in cookie.

Also, an error will be produced if colspan attribute for a custom cell will not be exist.

First table of three in a form

Actually there should be 3, since weights columns are hidden. So JS logic should just decrease it by one.

Second table of three in a form

Last table in a form

Extra-row without colspan attribute

CommentFileSizeAuthor
#15 drupal-tabledrag_custom_row_failing_cases-2836827-15.patch6.47 KBBR0kEN
#11 drupal-tabledrag_custom_row_failing_cases-2836827-11.patch2.88 KBBR0kEN
#2 drupal-tabledrag_custom_row-2836827-2.patch3.39 KBBR0kEN
tabledrag-4.png108.56 KBBR0kEN
tabledrag-3.png166.8 KBBR0kEN
tabledrag-2.png164.36 KBBR0kEN
tabledrag-1.png180.6 KBBR0kEN
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

BR0kEN created an issue. See original summary.

BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented
Assigned: BR0kEN » Unassigned
Status: Active » Needs review
FileSize
drupal-tabledrag_custom_row-2836827-2.patch3.39 KB
BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented

Just couple explanations of the changes.

  1. +++ b/misc/tabledrag.js
@@ -134,29 +134,19 @@ Drupal.tableDrag.prototype.initColumns = function () {
+          if (n < index && this.colSpan > 1) {

    colSpan - is default property of HTMLTableCellElement 

  2. +++ b/misc/tabledrag.js
@@ -134,29 +134,19 @@ Drupal.tableDrag.prototype.initColumns = function () {
+        cell = cells.eq(index);

    .eq() is simpler than :nth-child(). 

  3. +++ b/misc/tabledrag.js
@@ -174,7 +164,7 @@ Drupal.tableDrag.prototype.initColumns = function () {
+      this.showColumns(false);

    No need to increase column spans initially. 

  4. +++ b/misc/tabledrag.js
@@ -192,8 +182,8 @@ Drupal.tableDrag.prototype.hideColumns = function () {
+  $('.tabledrag-has-colspan', this.table).each(function () {

    Use the context of table to not process all spans of all tables with "tabledrag".

BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented 
+++ b/misc/tabledrag.js
@@ -134,29 +134,19 @@ Drupal.tableDrag.prototype.initColumns = function () {
-          if (cell[0].colSpan && cell[0].colSpan > 1) {

Could be a case when cell[0] is undefined.

Status: Needs review » Needs work

The last submitted patch, 2: drupal-tabledrag_custom_row-2836827-2.patch, failed testing.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
CreditAttribution: benjifisher at Isovera commented

Is there an easy way to reproduce this problem? For example, do your screenshots show a form generated by Panels or some related module?

Also, for every table manually added one extra row, containing a custom button in a single column. For this column, in last row, the colspan attribute is set to 4 to make it as wide as row is.

Can you show us the code that "manually" adds the extra row? When you say "the colspan attribute is set to 4", do you mean that it is defined this way in the PHP code or do you mean that the tableDrag javascript sets it?

if (undefined === updateColSpans ? true : updateColSpans) { ... }

Too bad we cannot use EC6, yet, because then we could supply default parameters much like in PHP. I think this could be simplified a bit:

if (undefined === updateColSpans || updateColSpans) { ... }
jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

Just for reference, the 'CI error' from #2 is a testbot script fault and the issue is #2837015: CI problem with permission when testing 7.x patches Assigned to: Mixologic but no-one is working on fixing it yet.

BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented
Status: Needs work » Needs review
BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented

@benjifisher, where you've seen the ES6 features in the undefined === updateColSpans ? true : updateColSpans construction?

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
CreditAttribution: benjifisher at Isovera commented

@BR0kEN, what I meant is that ES6 allows

function (updateColSpans = false) {
  // ...
  if (updateColSpans) { ... }
}

instead of

function (updateColSpans) {
  // ...
  if (undefined === updateColSpans ? true : updateColSpans) { ... }
}
BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented
FileSize
drupal-tabledrag_custom_row_failing_cases-2836827-11.patch2.88 KB

This patch should not be committed! It was added only for reproducing the error.

  1. Build a sandox: https://simplytest.me/project/drupal/7.x?add[]=table_element&patch[]=htt...
  2. Enable the system_tabledrag_test module.
  3. Visit the test/system/tabledrag/without-colspan and check TypeError: cell[0] is undefined JS error in developer tools.
  4. Visit the test/system/tabledrag/with-colspan and check that every table contains different value for colspan attribute in column of custom row.

Please note, that table_element module is used only for adding colspan attribute column in custom row. Without it behavior even worse (JS error).

BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented

Status: Needs review » Needs work

The last submitted patch, 11: drupal-tabledrag_custom_row_failing_cases-2836827-11.patch, failed testing.

BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented
Status: Needs work » Needs review

And, to see the result, build a sandbox using next link: https://simplytest.me/project/drupal/7.x?add[]=table_element&patch[]=htt.... Don't forget to enable system_tabledrag_test module again.

  1. On the test/system/tabledrag/without-colspan page, column of extra row will not have colspan attribute at all. No JS errors will appear.
  2. On the test/system/tabledrag/with-colspan everything will be fine.
BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented
FileSize
drupal-tabledrag_custom_row_failing_cases-2836827-15.patch6.47 KB

The same as in #11, but with Behat tests (TqExtension requirements should be complied).

  1. <BASE_URL> in behat/behat.yml should be replaced by Drupal base URL.
  2. <DRUPAL_ROOT> in behat/behat.yml should be replaced by value of DRUPAL_ROOT PHP constant.
  3. Composer dependencies have to be installed.
BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented
BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented

Needs attention.

BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and for FFW commented

Still...

caspervoogt’s picture

caspervoogt CreditAttribution: caspervoogt at Plethora commented

I was running into some tabledrag.js errors on a custom module of ours, and applied patch #2, and it solved our issue. Have not tried the patch from #15.

albertski’s picture

albertski CreditAttribution: albertski at Xeno Media, Inc. commented

I was getting errors only in IE for this.colSpan = this.colSpan - 1 in the Reduce the scolspan of any effected multi-span columns section.

Applying patch #2 fixes my issue. I think patch #15 only contains tests.