If I do

$variables = array(
  'items' => array(
    'Item one',
    'Item two',
    'Item three',
  ),
);

print theme('item_list', $variables);

$variables = array(
  'items' => array(
    'one' => 'Item one',
    'two' => 'Item two',
    'three' => 'Item three',
  ),
);

print theme('item_list', $variables);

The result will be:

<div class="item-list">
  <ul>
    <li class="first">Item one</li>
    <li>Item two</li>
    <li class="last">Item three</li>
  </ul>
</div>

<div class="item-list">
  <ul>
    <li class="first">Item one</li>
    <li class="first">Item two</li>
    <li class="first">Item three</li>
  </ul>
</div>

When using associative arrays, every item has the "first" class, and the "last" class is missing on the last one.

The documentation says nothing about associative arrays on the items parameter, so I think this is a bug that must be fixed. Otherwise we must fix the docs to clarify that only indexed arrays are allowed.

PS: I think this issue doesn't affect Drupal 8 due to the refactorings done at #256827: Various bugs in theme_item_list()

Files: 
CommentFileSizeAuthor
#5 0001-Issue-1809836-theme_item_list-is-broken-when-items-v.patch2.63 KBdanillonunes
PASSED: [[SimpleTest]]: [MySQL] 39,504 pass(es). View
#3 0001-Improved-the-item-list-tests.patch1.87 KBdanillonunes
FAILED: [[SimpleTest]]: [MySQL] 39,511 pass(es), 1 fail(s), and 0 exception(s). View
#4 0001-Issue-1809836-theme_item_list-is-broken-when-items-v.patch3.08 KBdanillonunes
PASSED: [[SimpleTest]]: [MySQL] 39,498 pass(es). View
#1 0001-Issue-1809836-theme_item_list-is-broken-when-items-i.patch1.45 KBdanillonunes
PASSED: [[SimpleTest]]: [MySQL] 39,523 pass(es). View

Comments

danillonunes’s picture

Title: theme_item_list is broken with associative arrays » Issue #1809836: theme_item_list() is broken when items is a associative array
Status: Active » Needs review
FileSize
1.45 KB
PASSED: [[SimpleTest]]: [MySQL] 39,523 pass(es). View

Didn't do the tests locally, but I hope there's no test expecting this bugged behaviour.

danillonunes’s picture

Title: Issue #1809836: theme_item_list() is broken when items is a associative array » theme_item_list() is broken when "items" variable is a associative array
danillonunes’s picture

FileSize
1.87 KB
FAILED: [[SimpleTest]]: [MySQL] 39,511 pass(es), 1 fail(s), and 0 exception(s). View

I also improved the tests to cover this case. This patch has just the test change, so now it must no pass.

danillonunes’s picture

FileSize
3.08 KB
PASSED: [[SimpleTest]]: [MySQL] 39,498 pass(es). View

And this patch is #1 and #3 merged, aka the patch that must be commited if everything is ok.

danillonunes’s picture

FileSize
2.63 KB
PASSED: [[SimpleTest]]: [MySQL] 39,504 pass(es). View

Actually, the last test is redundant now and can be removed.

wundo’s picture

Status: Needs review » Reviewed & tested by the community

I've tested locally is works perfectly.

Next time could you please name the patch file like this: [description]-[issue-number]-[comment-number].patch? http://drupal.org/node/1054616

danillonunes’s picture

Well, so item 1 is the first and item *total number of items* is the last. I don't have to do a weird thing like $i = -1; at start just so I could use a weird if ($i == $num_items - 1) at the end.

webchick’s picture

Assigned: Unassigned » David_Rothstein

Hm. Not sure about this one. Assigning to David.

webchick’s picture

Version: 7.x-dev » 8.x-dev
Assigned: David_Rothstein » Unassigned
Status: Reviewed & tested by the community » Patch (to be ported)

Oh, no I'm not. :) We need a 8.x patch first.

danillonunes’s picture

Version: 8.x-dev » 7.x-dev
Status: Patch (to be ported) » Reviewed & tested by the community

@webchick I'm reverting the changes because 8.x was already fixed by #256827: Various bugs in theme_item_list() :)

webchick’s picture

Assigned: Unassigned » David_Rothstein

Ah, sorry! Missed that. David it is then. :) We don't normally change theme functions in stable releases. This particular case looks fairly harmless, but David is really good at sniffing out "harmless" from actual harmless. ;D

David_Rothstein’s picture

Title: theme_item_list() is broken when "items" variable is a associative array » theme_item_list() is broken when "items" variable is an associative array (followup for tests)
Version: 7.x-dev » 8.x-dev
Assigned: David_Rothstein » Unassigned
Category: bug » task
Status: Reviewed & tested by the community » Active
Issue tags: +7.17 release notes

Looks good to me.

However, the code in Drupal 8 behaves a bit differently from this, and as far as I can see nothing like the test additions in this patch were ever added to Drupal 8 (so this functionality doesn't actually seem to be tested there). That's not so good; I think the tests should be forward-ported.

But in the meantime, committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/ff1d0fc

Cottser’s picture

Title: theme_item_list() is broken when "items" variable is an associative array (followup for tests) » theme_item_list()/item-list.html.twig is broken when "items" variable is an associative array (followup for tests)
Issue summary: View changes
Issue tags: +rc eligible, +Needs tests

If we're just adding tests it's eligible for RC.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.