Opened 9 months ago
Closed 7 months ago
#63005 closed defect (bug) (fixed)
Replacing certain bulk-edit fields blocks all bulk edits
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 6.8.1 | Priority: | normal |
| Severity: | normal | Version: | 6.7 |
| Component: | Administration | Keywords: | has-patch fixed-major dev-feedback |
| Focuses: | javascript, administration | Cc: |
Description
In r59134, validation was added to prevent users from submitting bulk edits if no items are selected, or if no change is chosen for the selected items. As part of the validation, a specific input name is expected for the new value. The input name cannot be customized.
If, for example, the "Change role to..." dropdown on the WP_Users_List_Table is removed, validation for all other bulk actions will fail because the validation requires an input named new_role. I encountered this when replacing the default dropdown with a multi-select to support Core's ability for users to have multiple roles. I was able to work around this issue by including a hidden input with the expected name, but that defeats the validation.
If the input names could be modified, the validation would support custom input names without requiring extra, unused hidden fields; developers would also not need to implement their own client-side validation. Currently, the input names are hardcoded in the validation JS, but they could be passed via localization after being filtered.
Attachments (2)
Change History (16)
#1
@
9 months ago
- Milestone changed from Awaiting Review to 6.7.3
- Version set to 6.7
cc @joedolson - This relates to the work you did in 6.7
While I don't think this should be trigger a 6.7 minor in and of itself, I think that depending on the fix, this could make sense to backport to the 6.7 branch.
#2
@
9 months ago
- Owner set to joedolson
- Status changed from new to accepted
@jorbin If there's any likelihood we'll have an additional minor for 6.7, then it probably does make sense to backport.
@kabir93 I think it would make sense to add in the additional change in user.js; I don't see a lot of value in separating those into two separate commits.
#3
@
9 months ago
I think the approach in 63005.diff makes a little more sense to me since the fix isn't limited to the user table and could provide flexibility to the posts screen as well.
@joedolson My gut says it's highly unlikely that there would be an additional minor while 6.7 is the supported version, but I also think that having things tested and ready in the branch makes it easier if we decide to do one.
#4
@
8 months ago
Makes sense to me to go with the simpler version.
@ethitter The patch looks good in principle, but I think we could do this in a single filter by filtering the whole array, and I think that would be preferable - I don't think it would be a common scenario where somebody would only want to change one of the parameters, so they might as well be filtered together.
We'll also need a doc block for the filter.
This ticket was mentioned in PR #8536 on WordPress/wordpress-develop by @jorbin.
8 months ago
#5
This updates the patch from @ethitter to include inline documentation and a single filter
#6
@
7 months ago
- Milestone changed from 6.7.3 to 6.8.1
Since 6.8 is imminent, going to punt this to 6.8.1 as no 6.7.3 is planned at this time.
Attached
63005.alternative.diffas an alternative approach. This patch: - Adds a filterablebulk_edit_input_nameinWP_Users_List_Tablewith a context parameter for flexibility. - Localizes the input name toBulkEditSettingsinuser.js, keeping the solution specific to the Users screen. - Avoids modifyingcommon.js, reducing the risk of side effects on other list tables. Compared to63005.diff, this offers a more targeted fix, simpler maintenance, and a reusable filter design. It assumes a follow-up change inuser.jsto useBulkEditSettings.inputName(not included here but can be provided). Feedback welcome!