WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#15922 closed defect (bug) (fixed)

WP_MS_Themes_List_Table plural

Reported by: scribu Owned by: ocean90
Milestone: 3.1 Priority: normal
Severity: minor Version:
Component: Security Keywords: needs-patch
Focuses: Cc:

Description

Currently, the plural is set to 'plugins'. This is a hack to avoid duplicating CSS.

The problem is that the plural is used for generating the nonce too.

The attached patch limits the hack to only the CSS class, allowing the correct nonce to be generated and checked.

Attachments (2)

15922.diff (731 bytes) - added by scribu 4 years ago.
15922.css1.patch (5.8 KB) - added by ocean90 4 years ago.

Download all attachments as: .zip

Change History (15)

@scribu4 years ago

comment:1 @westi4 years ago

Can we not just add themes to the CSS file in the relevant places too?

comment:2 @ocean904 years ago

  • Owner set to ocean90
  • Status changed from new to assigned

@ocean904 years ago

comment:3 @ocean904 years ago

15922.css1.patch is the first pass.

(Discussion on IRC)

comment:4 @scribu4 years ago

I hesitated to add the styles, since I think it would be better if we had a single, generic 'themesorplugins' class (with a better name obviously).

comment:5 follow-up: @ocean904 years ago

Decision from scribu, JohnONolan and me. Keep the .plugins class and add instead of the .themes class a more generic class name, which wil will use for the plugins and themes list. Suggestions?

(Sitenote: We need to find a way to deprecate CSS).

Version 0, edited 4 years ago by ocean90 (next)

comment:6 in reply to: ↑ 5 @ryan4 years ago

Replying to ocean90:

Decision from scribu, JohnONolan and me. Keep the .plugins class and add instead of the .themes class a more generic class name, which wil will use for the plugins and themes list. Suggestions?

Fine by me.

comment:7 @ryan4 years ago

Per IRC discussion, we're going with the simple, conservative 15922.diff patch and whatever nonce fixes that requires.

comment:8 @nacin4 years ago

I believe 15922.diff fixes the nonces, yes?

comment:9 @scribu4 years ago

The 'bulk-themes' nonce is already set in /network/themes.php but it doesn't seem to be checked anywhere. If it were, there would be some errors, without this patch.

comment:10 @ryan4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [17115]) Set the correct plural. Use plugins class on the table. Props scribu. fixes #15922

comment:11 @scribu4 years ago

  • Keywords needs-patch added; has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

As stated above, the nonce check is missing.

comment:12 @ryan4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [17121]) Check bulk-themes nonce before bulk delete. fixes #15922

comment:13 @ryan4 years ago

There are other places that need nonces. We can audit them in a dedicated ticket.

Note: See TracTickets for help on using tickets.