Opened 4 years ago
Closed 4 years ago
#50500 closed defect (bug) (fixed)
Customize: JavaScript error breaks Customizer when loading locked changeset
Reported by: | dlh | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | major | Version: | 5.3 |
Component: | Customize | Keywords: | has-patch |
Focuses: | Cc: |
Description
The Customizer includes a locking mechanism to prevent two users from editing the same changeset simultaneously. Currently, when opening a changeset while another user is editing it, a JavaScript error occurs when showing the "changeset is locked" overlay. The overlay does not appear, leaving the Customizer in a broken state.
To replicate, an installation with two administrator accounts is needed:
- Open the Customizer while logged in as the first administrator. Make a change and save a draft of the changeset. Leave this window open.
- In an incognito window, log in as the second administrator and open the Customizer.
The expected behavior is that an overlay appears notifying the second administrator that another user is customizing the site. Instead, a JavaScript error appears in the console.
I believe the notification was broken in #45066 for WordPress 5.3.
The Customizer's LockedNotification
, which notifies the user that the changeset is locked, does not specify a default message
, which means it uses the value of null
from the base Notification
class.
As a result, the message
passed to wp.a11y.speak()
for the notification created by startLock()
is also null
: https://github.com/WordPress/wordpress-develop/blob/7b28cdbdee351ddf0c580b8a2d68050ee90583b8/src/js/_enqueues/wp/customize/controls.js#L253
Prior to 5.3, the wp.a11y.speak()
JS function forgivingly handled empty non-string input via wp.sanitize.stripTags()
: https://github.com/WordPress/wordpress-develop/blob/7b28cdbdee351ddf0c580b8a2d68050ee90583b8/src/js/_enqueues/wp/sanitize.js#L24
The current a11y
package does not provide the same fallback: https://github.com/WordPress/gutenberg/blob/969d26f925896dce12a65cdaf06d248722970b38/packages/a11y/src/filter-message.js#L18
The attached patch ensures that the LockedNotification
provides an appropriately typed default message
parameter for the new package.
Whether these notifications should attempt to speak an empty value is a worthwhile question, but this ticket aims to limit its scope only to getting the Customizer back in action.
Arguably, the default message
on the base Notification
class should instead be a string instead of null
, which would also address the issue.
I have not done so yet in this patch, again for reasons of scope, although I would be open to assurances that the change would be safe.
In 48210: