List table screens in network admin break if no item is selected for bulk actions

[PeteMall]

All list table screens in net admin other than plugins and site-themes break when you select a bulk action and don't select any items.

[SergeyBiryukov]

Fixed notices on network/sites.php and network/users.php.

[ryan]

Patch needs to be refreshed to handle the recently added delete-selected case.

[SergeyBiryukov]

Revised the patch.

[scribu]

The messages don't distinguish between individual actions and bulk actions, i.e., if I delete two themes, I still get "Theme deleted".

[nacin]

We should pass the number of themes affected back and use _n().

This can either be a new query arg (n=1, n=5), or just enabled=1 or enabled=5, versus the generic update.

[nacin]

(In [17128]) Prevent notices in the allblogs and allusers branches. props SergeyBiryukov, see #15920.

[SergeyBiryukov]

Added distinguishing between individual actions and bulk actions.

[johnpbloch]

Just tested against trunk. Everything checks out and works as expected (patch 15920-take3.patch).

[nacin]

New patch:

• Removes apparently bogus esc_html() wrappers around $theme['Stylesheet'] in WP_MS_Themes_List_Table. This is the directory name so it should be pretty safe already, and every usage of it ends up in esc_url() or esc_attr(). If for some reason esc_html( $theme_key ) != $theme_key though, this would pose quite a number of problems, as the database would be improperly updated.

• Hides the 'Delete' link for a theme active on the main site.

• Prevents theme deletion for a theme active on the main site.

• Properly resets the request URI arguments.

• Switches to more direct enabled=N (etc) messages. Also doesn't show "1" in the string when only one theme is deleted.

I'm still not a fan of this direct-to-DB logic, but I'll let ryan weigh in on the necessity of reworking that into a saner check.

			$allowed_themes[ $_GET['theme'] ] = true;
			update_site_option( 'allowedthemes', $allowed_themes );

[SergeyBiryukov]

Replying to nacin:

Also doesn't show "1" in the string when only one theme is deleted.

There's actually a slight i18n issue. It's good for English, however in Russian a single form is used for every number ending in 1 (except for 11): 1, 21, 31, etc. Standalone single form (as in “Theme enabled”) only makes sense for 1 item. In other cases (21, 31, ...) the number should be included in the string (as in “%s theme enabled”), but it's still single form. Currently there's no way to do this properly.

So my idea was to use “Theme enabled” for individual actions and “%s theme enabled” for bulk actions if only one item has been selected.

This is probably should go in another ticket, since there are some other messages concerned (e.g. “Item permanently deleted”). I'll create one if I get around to it.

[nacin]

Replying to SergeyBiryukov:

This is probably should go in another ticket, since there are some other messages concerned (e.g. “Item permanently deleted”). I'll create one if I get around to it.

Indeed, not a regression, so should be handled elsewhere.

[nacin]

According to the gnu gettext manual, that should be handled by a proper Plural-Forms header in your PO file. Thus, 'Item deleted' or 'One item deleted' is fine, as it handles the singular case, and you are then given as many plural forms as you need based on the logic in the Plural-Forms header. Not my area of expertise, but that makes sense to me.

[nacin]

site-themes.php might need some of the logic in this patch, based on a quick glance.

See also #15974 -- it needs some nonce checks too.

[nacin]

Looks like the REQUEST_URI bit might be needed for non-redirect reasons.

[SergeyBiryukov]

Replying to nacin:

According to the gnu gettext manual, that should be handled by a proper Plural-Forms header in your PO file. Thus, 'Item deleted' or 'One item deleted' is fine, as it handles the singular case, and you are then given as many plural forms as you need based on the logic in the Plural-Forms header.

I've been using the header from that same manual:

Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11) ? 0 : ((n%10>=2 && n%10<=4 && (n%100<10 || n%100>=20)) ? 1 : 2);

Perhaps it can be modified to handle this. Thanks for the hint!

[nacin]

I don't know enough to be particularly helpful here, but ​http://www.gnu.org/software/gettext/manual/html_node/Plural-forms.html specifically talks about the optional use of 'One' (no %d) in a singular form, and how that's perfectly fine for languages with advanced plural forms.

[nacin]

Patch includes better REQUEST_URI reset.

[ryan]

(In [17130]) MS themes fixes. Fix bulk actions when no items selected. Props SergeyBiryukov, nacin. fixes #15920

[PeteMall]

Better messages for site-themes.

[SergeyBiryukov]

Replying to nacin:

I don't know enough to be particularly helpful here, but ​http://www.gnu.org/software/gettext/manual/html_node/Plural-forms.html specifically talks about the optional use of 'One' (no %d) in a singular form, and how that's perfectly fine for languages with advanced plural forms.

I was able to sort it out by adding an extra rule for a single item and updating translations. It's great knowing that there's a simple solution if you look at the right place. Thanks again!

[westi]

(In [17137]) Improved messaging for theme enabled/disabled on a per-site bases. Fixes #15920 props PeteMall

[westi]

Looked great to me.

[nacin]

(In [17138]) Remove redundancy in these arguments. props PeteMall, fixes #15920.