"Show/hide descriptions" links no longer work

This patch is a partial fix. Also there's a #220263: Generalize "Hide descriptions" link

This should be a permanent cookie like the toolbar collapsed default rather than saved in user->data.

@catch I think a cookie is not a permanent storage for users' settings. While we still have {user}->data this setting should live here, same as contact.module setting. User could visit /admin from different computers and should have the same admin-screen

I don't consider this a setting, it's rather a toggle. If we say that, then the toolbar should be using $user->data too.

I think it'a good idea to separate UI settings and functional settings in different storage.

A few things:

I don't understand the logic behind this:

$edit['data'] = !empty($edit['data']) ? array_merge($edit['data'], $account->data) : $account->data;

Why do we clobber $edit['data'] when it's non-empty? It seems to me that I would want to clobber $account->data:

$edit['data'] = !empty($edit['data']) ? array_merge($account->data, $edit['data']) : $account->data;

Indeed comment #57 at #721436: Remove magical fairy saving of cruft from user_save() does this switch, so at least one other person agrees.

Also, I haven't been following #721436: Remove magical fairy saving of cruft from user_save(), but it seems to be a known issue that $account->data is not always unserialized. This is a problem because array_merge will complain unless $account->data is unserialized. In this patch, I unserialize $account->data if it needs to be.

Anyway, this patch fixes the current issue. I don't want to get mixed up in #721436: Remove magical fairy saving of cruft from user_save(), but someone from there can come here and edit my patch to fit whatever develops in that other issue.

I agree that this is and ephemeral pref like toolbar collapsed or the filter peristance on admin/content. so, this can be persisted in a cookie IMO. We have to move code out of $user->data anyway for D8, and might as well start now. We could do own cookie or re-use the drupal.visitor namespace.

Woow I was wondering this week if I had to open an issue about remembering the state of the hide descriptions...
But apparently this discussion is alrdy started. A big plus for the idea.

I don't care if it's a cookie or not.

Re-did it using Drupal.visitor cookie.

Fix missing semi-colon. By the way, do we really need a variable_get here? Will there really be a site administrator somewhere who would want to set admin_compact_mode to TRUE by default?

Re-roll #12 with tests.

Looks good to me.

for me too

+++ modules/system/system.module
@@ -2717,8 +2721,7 @@ function system_admin_compact_mode() {
-  user_save($user, array('admin_compact_mode' => ($mode == 'on')));
+  user_cookie_save(array('admin_compact_mode' => ($mode == 'on')), array('admin_compact_mode'));

user_cookie_save() has such a weird function signature. The fact that it takes two arrays is just awkward, IMO. Anyway, not related to this issue.

Committed #14 to CVS. I fully agree that we should use cookies for this, instead of a database setting.

BTW, I don't understand how this test passed without calling parent::setUp()?

@mfb because no moduless need to install

hmm but doesn't setUp() always have to call parent::setUp() to set up the database and testing environment?