Opened 17 months ago
Closed 16 months ago
#9003 closed defect (bug) (fixed)
The script assigned the handle of 'bp-rewrites-ui' fails to enqueue for the WordPress 5.8 branch [BP 12 beta3]
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 12.0.0 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Administration | Keywords: | has-patch |
Cc: |
Description
With BP 12.0.0-beta3 and WP 5.8.7, the script assigned the handle 'bp-rewrites-ui' (bp-core/admin/js/rewrites-ui.js) fails to enqueue, resulting in the inability of any given accordion to unhide its respective content when on the BP Settings - bp-rewrites page (wp-admin/admin.php?page=bp-rewrites). Meaning, this condition is unique to BP 12 and WP 5.8, n.b., WP branches higher than 5.8 don't have this issue.
That stated, there seems to be an appearance of a timing issue as to when the script is registered and when it is enqueued. For WP 5.8.7 there appears to be an attempt to enqueue the script before is has a chance to be registered. The call to wp_enqueue_script( 'bp-rewrites-ui' )
resides in the function "bp_core_admin_rewrites_load" (found in bp-core/admin/bp-core-admin-rewrites.php) which, is a callback to action hook "load-{$hook}", e.g., add_action( "load-{$hook}", 'bp_core_admin_rewrites_load' )
[found in bp-core/classes/class-bp-admin.php] , the $hook equates to the string 'settings_page_bp-rewrites'. The script is registered in in the class method "admin_register_scripts" (found in bp-core/classes/class-bp-admin.php) which, is a callback to the action hook 'bp_admin_enqueue_scripts', e.g., add_action( 'bp_admin_enqueue_scripts', array( $this, 'admin_register_scripts' ), 1 )
[found in bp-core/classes/class-bp-admin.php].
The following is an abbreviated load order for action hooks, n.b., the BasePress plugin is activated but, has no bearing on the isssue:
bp_admin_init
bp_register_importers
bp_register_admin_style
bp_register_admin_settings
delete_transient_basepress_flush_rules
basepress_settings_sections
basepress_settings_fields
current_screen
load-settings_page_bp-rewrites -> enqueue script (bp_core_admin_rewrites_load())
admin_xml_ns
admin_enqueue_scripts
bp_admin_enqueue_scripts -> register script (BP_Admin->admin_register_scripts())
Given this load order. it makes sense, at least to me, that the script would not enqueue since it would not have been registered at the time of enqueuing. The mystery, at least to me, is why this load order works for the branches of WP 5.9 and higher other than there was enough of a change in code such that this tactic works, however, it is not readily transparent to me, at this point in time.
Neither here nor there, What I did to ensure the script was registered before it was enqueued (based on the WP 5.8 branch) was to change one line of code:
Change line 176 of bp-core/classes/class-bp-admin.php:
From:
add_action( 'bp_admin_enqueue_scripts', array( $this, 'admin_register_scripts' ), 1 );
To;
add_action( 'bp_admin_init', array( $this, 'admin_register_scripts' ), 1 );
Thus, the load order is now (works for branches WP 5.9 & higher, as well:
bp_admin_init -> register script (BP_Admin->admin_register_scripts())
bp_register_importers
bp_register_admin_style
bp_register_admin_settings
delete_transient_basepress_flush_rules
basepress_settings_sections
basepress_settings_fields
current_screen
load-settings_page_bp-rewrites -> enqueue script (bp_core_admin_rewrites_load())
admin_xml_ns
admin_enqueue_scripts
bp_admin_enqueue_scripts
Given there is confirmation as to what I've attempted to describe as an issue (see attachment for screenshot for visual) and, since there are a number of ways, in which, to fix this issue, I'll leave it up to the discretion of whomever to apply an appropriate fix.
Attachments (1)
Change History (5)
#1
@
17 months ago
- Component changed from Core to Administration
- Milestone changed from Awaiting Review to 12.0.0
Thanks a lot for your detailed report @emaralive
I'll look at it asap.
#2
@
17 months ago
- Keywords needs-patch added
I confirm the issue, I'll work on a fix asap. I agree it's weird it's enqueued in higher branches than 5.8.
I'll enqueue the script in the bp_core_admin_rewrites_settings()
function once we're sure script is registered.
This ticket was mentioned in PR #175 on buddypress/buddypress by @imath.
17 months ago
#3
- Keywords has-patch added; needs-patch removed
Wait for 'bp-rewrites-ui' JS to be registered before enqueuing it.
Trac ticket: https://buddypress.trac.wordpress.org/ticket/9003
Screenshot of BuddyPress Settings - bp-rewrites page