Make WordPress Core

Opened 10 years ago

Closed 4 years ago

Last modified 4 years ago

#39451 closed defect (bug) (fixed)

Javascript error when link_manager_enabled ( WP_Widget_Links) and theme supports customize-selective-refresh-widgets

Reported by: nikeo Owned by: westonruter
Priority: normal Milestone: 6.1
Component: Customize Version: 4.5
Severity: normal Keywords: has-patch commit
Cc: Focuses: javascript

Description

The error occurs in the customizer when a user uses the WP_Widget_Links widget has enabled support for customize-selective-refresh-widgets.

The WP_Widget_Links is now deprecated, but it is retro-compatible if user had it already installed.

The error message in the browser js console looks like this :

customize-preview-widgets.js?ver=4.7:63 Uncaught Error: Illegal id for widget partial.
    at child.initialize (customize-preview-widgets.js?ver=4.7:63)
    at child.api.Class (customize-base.js?ver=4.7:93)
    at child [as constructor] (customize-base.js?ver=4.7:34)
    at new child (customize-base.js?ver=4.7:34)
    at HTMLDivElement.<anonymous> (customize-selective-refresh.js?ver=4.7:863)
    at Function.each (jquery.js?ver=1.12.4:2)
    at jQuery.fn.init.each (jquery.js?ver=1.12.4:2)
    at Object.wp.customize.selectiveRefresh.self.addPartials (customize-selective-refresh.js?ver=4.7:850)
    at Function.<anonymous> (customize-selective-refresh.js?ver=4.7:961)
    at i (jquery.js?ver=1.12.4:2)

Theme or plugin developers can fix the problem by removing the support for customize-selective-refresh-widgets if link_manager_enabled is set to true. But a core fix would be better of course.

Temporary solution :

if ( ! get_option( 'link_manager_enabled' ) ) {
   add_theme_support( 'customize-selective-refresh-widgets' );
}

Attachments (1)

39451.diff​ (685 bytes ) - added by dlh 9 years ago.

Download all attachments as: .zip

Change History (25)

#1 @nikeo
10 years ago

There's a typo in my description. I meant : The error occurs in the customizer when a user uses the WP_Widget_Links widget and has enabled support for customize-selective-refresh-widgets.

Last edited 10 years ago by nikeo (previous) (diff)

#2 @westonruter
10 years ago

  • Keywords needs-patch added
  • Milestone Awaiting Review → Future Release

Coincidentally I also ran across this issue myself in working on the ​JS Widgets plugin for #33507 and #35574.

#3 @westonruter
10 years ago

  • Version 4.7 → 4.5

#4 @nikeo
10 years ago

OK.
The bug was reported by two different users for my theme Hueman.
The latest topic is this one : ​https://wordpress.org/support/topic/customizer-never-loads-in-chrome-with-hueman-addon-plugin-enables/
cheers

#5 @greenshady
9 years ago

I've run into this problem too with users. I've also seen it within a plugin of mine that creates a more robust links widget.

I think this is because the links widget creates "fake" widgets on output. Each category of links is wrapped into its own widget markup. And, the customizer JS is looking to match actual widgets.

Even disabling the customize_selective_refresh for the Links widget doesn't address the issue.

This ticket was mentioned in ​Slack in #core-customize by presskopp. ​View the logs.


9 years ago

@dlh
9 years ago

#7 @dlh
9 years ago

  • Keywords has-patch added; needs-patch removed

WP_Widget_Links::widget() contains a preg_replace() for the id attribute that also seems to be affecting the data-customize-partial-id attribute, creating the "illegal id." The fix might be as straightforward as adjusting the regex pattern, which I've tried in 39451.diff​.

#8 @rayfusci
8 years ago

This 39451.diff patch is not present in v4.9.8. Any plans to include it? It does resolve this problem, at least in some cases; it resolved this problem in my case.

Last edited 8 years ago by rayfusci (previous) (diff)

#9 @dlh
8 years ago

#45314 was marked as a duplicate.

#10 @Jonas Lundman
5 years ago

This 39451.diff patch is not present in latest version of WP 5.8. Why isnt it fixed?

The default patch in the widget replaces any theme "before widget" settings with a placeholder %s for the category ID, NOT the link-nr instance (but the latter is what customizer need).

This widget actually output the whole widget markup for each categories, - each category is a widget (!), that idea creates multiple same id if "before widget" args are use.

So decide, keep customizer broken, or fix by keep multiple id:s, or adjust the widget correctly.

Last edited 5 years ago by Jonas Lundman (previous) (diff)

#11 @dlh
5 years ago

  • Milestone Future Release → 6.0

This ticket was mentioned in ​Slack in #core by mike. ​View the logs.


4 years ago

#13 @sumitsingh
4 years ago

  • Keywords needs-testing added

This ticket was mentioned in ​Slack in #core by costdev. ​View the logs.


4 years ago

This ticket was mentioned in ​Slack in #core by chaion07. ​View the logs.


4 years ago

#17 @costdev
4 years ago

  • Keywords commit added; needs-testing removed

Test Report

Environment

  • Server: Apache (Linux)
  • WordPress: 6.0-beta4-53335-src
  • Browser: Chrome 101.0.4951.41
  • OS: Windows 10
  • Theme: Twenty Twenty-One
  • Plugins: None activated.

Steps

  1. Save the following to wp-content/plugins/customize-selective-refresh-widgets.php:
    <?php
    
    /**
     * Plugin Name: customize-selective-refresh-widgets
     * Description: customize-selective-refresh-widgets
     * Author:      WordPress Core Contributors
     * Author URI:  https://make.wordpress.org/core
     * License:     GPLv2 or later
     * Version:     1.0.0
     */
    
    add_action(
            'after_setup_theme',
            function() {
                    add_filter( 'pre_option_link_manager_enabled', '__return_true' );
                    add_theme_support( 'customize-selective-refresh-widgets' );
            }
    );
    
  2. Navigate to Plugins > Installed Plugins.
  3. Enable the customize-selective-refresh-widgets plugin.
  4. Navigate to Links > Add New.
  5. Enter a name and a web address. Click Add Link.
  6. Navigate to Appearance > Widgets.
  7. Add a Links widget to the Footer sidebar.
  8. Navigate to Appearance > Customize.
  9. Open up DevTools. Notice an error: "Uncaught Error: Illegal id for widget partial". ✅
  10. Apply 39451.diff.
  11. Refresh the Customizer page.
  12. Open DevTools. Notice the error is gone. ✅

Results

  1. Issue reproduced. ✅
  2. 39451.diff resolves the issue.
  3. No regressions identified.

Notes

  1. ​PR 2661 is a refreshed version of 39451.diff to show that CI passes.
  2. Adding commit.

This ticket was mentioned in ​Slack in #core by costdev. ​View the logs.


4 years ago

#19 @costdev
4 years ago

  • Keywords needs-refresh added; commit removed
  • Milestone 6.0 → 6.1

As 6.0 RC1 starts in less than an hour and this still needs confirmation that there are no side-effects, I'm moving this to the 6.1 milestone.

#20 @costdev
4 years ago

  • Keywords needs-testing added; needs-refresh removed

#21 @westonruter
4 years ago

  • Keywords commit added; needs-testing removed
  • Owner set to westonruter
  • Status new → accepted

The before_widget string being replaced:

<div  data-customize-partial-id="widget[links-2]" data-customize-partial-type="widget" data-customize-partial-placement-context="{&quot;sidebar_id&quot;:&quot;sidebar-1&quot;,&quot;sidebar_instance_number&quot;:1}" data-customize-widget-id="links-2" class="widget widget_links"><div class="widget-content">

Difference in $before_widget before/after:

- <div  data-customize-partial-id="%id" data-customize-partial-type="widget" data-customize-partial-placement-context="{&quot;sidebar_id&quot;:&quot;sidebar-1&quot;,&quot;sidebar_instance_number&quot;:1}" data-customize-widget-id="%id" class="widget widget_links"><div class="widget-content">
+ <div  data-customize-partial-id="widget[links-2]" data-customize-partial-type="widget" data-customize-partial-placement-context="{&quot;sidebar_id&quot;:&quot;sidebar-1&quot;,&quot;sidebar_instance_number&quot;:1}" data-customize-widget-id="links-2" class="widget widget_links"><div class="widget-content">

The preg_replace() in question here comes from WordPress 2.8 in [10795] to fix #9349, which is all a part of introducing WP_Widget. The %id string appears to be part of how wp_list_bookmarks() does its thing.

The patch fixes the error for me still.

#22 @westonruter
4 years ago

The more robust solution here will eventually be to utilize the proposed ​new system for simply and reliably updating HTML attributes.

#23 @westonruter
4 years ago

  • Resolution → fixed
  • Status accepted → closed

In 54125:

Customize: Prevent JS error in Links widget when selective refresh is enabled

This prevents erroneously replacing the data-customize-partial-id when only the id attribute should be replaced.

Props dlh, costdev, nikeo, greenshady.
Fixes #39451.

Note: See TracTickets for help on using tickets.