Regression: Draggable table broken on image style form

The #tabledrag is processed drupal_pre_render_table() and for some reason it is not getting called on this page (though theme_table() does), not sure why. it would be an issue with combination of theme_image_style_preview + drupal_pre_render_table

Doesn't look like a JS issue to me.

#3: I manually tested everything that was affected — see #732022-32: drupal_add_tabledrag() still using drupal_add_(js|library)(), should return array for #attached. Somehow this must've sneaked through the cracks.

The fix is trivial: use #type => table instead of #theme => table.

Almost looked at that line 10x and couldn't get that issue. Nice catch @Wim Leers :)

Hehe, you found the underlying problems with using #theme, you just didn't notice that tiny part :)

+++ b/core/modules/image/image.admin.inc
@@ -54,7 +54,7 @@ function theme_image_style_effects($variables) {
     '#rows' => $rows,
...
+    '#type' => 'table',

Don't want to un-RTBC this since this is a major bug, but generally the rows of a #type table are added as child elements of the table elements. This is the #theme table way of doing it.

#10: "generally" maybe though I personally didn't know about this, but if that's how it's intended to work, then #rows should simply be removed? :)

Re #11: Sorry, I should have explained that better:

Using #rows works and therefore #5 is fine and due to the issue status I support RTBC. However, #rows expects a certain data structure. Most importantly, it does not support render arrays as cells. This can be seen in theme_image_style_effects() in particular, as all of the cells have to be drupal_render()ed before being but into #rows. #type table would allow to simply leave those as render arrays and put them into $table[$row][$column]. This is preferable for the reason above and several others.

#12: I see :) Yeah, let's do that in a follow-up.

on the other note, we got 46 instance of #theme => table (on a rough search). Is it worth opening an issue with fixing them with #type => table where appropriate and a test case to make sure we are good?. Attaching the search result for reference.

#14: you may want to look at #2152193: Discuss the fate of #type table for that.

Hm. I can't think of a way to test this that doesn't special-case this one table, which doesn't make a whole lot of sense as a test. So I think this is good to go, despite lacking test coverage.

Committed and pushed to 8.x. Thanks!