When there are no content items under the current filter at /admin/content/node, the "select all" checkbox at the top should not be shown. There are no items to select, so select all doesn't make any sense. See patch. This probably applies to HEAD also.

Comments

lyricnz’s picture

Version: 5.1 » 6.x-dev

Moving to head, more likely to get applied there :) then backported to 5.1

lyricnz’s picture

StatusFileSize
new1.09 KB

Tested in 6.x. Fix typo.

lyricnz’s picture

StatusFileSize
new1.13 KB

Reroll and apply from drupal root.

pwolanin’s picture

code looks reasonable - but a little hard to read the inline conditional. Also - should the header cell get some content? a space or nbsp?- is it valid HTML to have it totally empty?

lyricnz’s picture

StatusFileSize
new1.17 KB

It's definitely valid to have empty TD/TH cells, and it's actually rendered differently to a non-empty cell. This especially shows if you have a border defined for your table. Not sure that matters here.

Anyway, I've extracted the inline ?: into variable. Patch attached.

pwolanin’s picture

Category: feature » bug
Status: Active » Needs review
gábor hojtsy’s picture

Why do we display the table at all, if there are no elements?

lyricnz’s picture

Gotta put the "there's no content" message somewhere, I guess, but good point.

I figured we should leave the table there, because as users fiddle with their filters, it might be confusing to see the table appear and disappear.

gábor hojtsy’s picture

The possible confusion about displaying the table or not displaying it sometimes seems to be a valid point for me.

Freso’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new1.18 KB

The patch doesn't apply:

gentoo-vm drupal6 # patch -p0 < node.module-select-all_2.patch
patching file modules/node/node.module
Hunk #1 FAILED at 1698.
Hunk #2 FAILED at 1708.
2 out of 2 hunks FAILED -- saving rejects to file modules/node/node.module.rej

So I re-rolled against latest HEAD and tested it, with the conclusion that the patch works as advertised. Marking RTBC.

gábor hojtsy’s picture

Status: Reviewed & tested by the community » Needs work

It would be good to document in a comment, why isset($form['title']) && is_array($form['title']) leads to $has_posts. It looks like an interesting conditional. Otherwise all seem to look fine.

DrupalTestbedBot tested Freso's patch (http://drupal.org/files/issues/node.module-select-all_3.patch), the patch passed. For more information visit http://testing.drupal.org/node/124

lyricnz’s picture

Agree, Gábor. We didn't create that condition, just moved it from further down. How about:

// If there are rows in this form, then $form['title'] contains a list of the title form elements.

lyricnz’s picture

FWIW the reason the patch didn't apply was that d6 has split node.module. I'll reroll a backport for d5 this is commited to d6.

Freso’s picture

lyricnz: As Gábor hasn't commented either way with regards to the comment, I think it'd be safe to make a new patch/re-roll the patch with the comment and mark this as "needs review".

lyricnz’s picture

Status: Needs work » Needs review
StatusFileSize
new1.32 KB

Added comment describing $has_posts.

gábor hojtsy’s picture

Status: Needs review » Fixed

Looks better, thanks, committed.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.