WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 5 months ago

#31517 closed enhancement (fixed)

Customizer: show a notice after attempting to navigate to external links in live previews

Reported by: designsimply Owned by: westonruter
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch needs-testing
Focuses: ui, javascript Cc:

Description (last modified by westonruter)

The Customizer only allows you to click internal, front-end links within a preview to navigate to URLs you can customize:
https://core.trac.wordpress.org/browser/trunk/src/wp-admin/js/customize-controls.js#L2110

Clicking on an external link from the live preview pane fails silently. To help avoid confusion, we could display a notice to alert when someone clicks on an external link in a live preview:

You can only preview internal, front-end links in the customizer.

Here is a past description of how a customizer notice could work from @westonruter: https://core.trac.wordpress.org/ticket/30677#comment:7 (different use case, but the part about how the alert might work is relevant)

#29098 provides a general mechanism for sending back error codes/messages.

Change History (12)

#1 @westonruter
2 years ago

  • Milestone changed from Awaiting Review to Future Release

I implemented this actually as part of my transactions proposal: #30937.

As in my pull request:

It outputs the following CSS to the Customizer preview:

.customize-preview-not-allowed,
.customize-preview-not-allowed * {
    cursor: not-allowed !important;
}

And the following JS is added to the Customizer preview as well:

$( 'form[action], a[href]' ).each( function () {
        var url, el = $( this );
        url = el.prop( 'href' ) || el.prop( 'action' );
        if ( url && ! self.isAllowedUrl( url ) ) {
                el.addClass( 'customize-preview-not-allowed' );
                el.prop( 'title', api.settings.l10n.previewNotAllowed );
        }
});

This ensures the tooltip and the proper cursor are displayed.

And then when actually trying to click on a link (or submit a form), it prevents the default action:

 this.body.on( 'click.preview', 'a', function( event ) {
        var to = $( this ).prop( 'href' );

        // @todo Instead of preventDefault and bailing, should we instead show an AYS/confirm dialog?

        if ( ! self.isAllowedUrl( to ) ) {
                event.preventDefault();
        }
});

this.body.on( 'submit.preview', 'form', function( event ) {
        var form = $( this );
        if ( ! self.isAllowedUrl( this.action ) ) {
                event.preventDefault();
                return;
        }
        // ...
});

Currently in Core, the logic for isAllowedUrl is handled in the Customizer pane. The transactions patch moves this logic to the preview instead.

#2 @westonruter
11 months ago

  • Milestone changed from Future Release to 4.6

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


11 months ago

#4 @celloexpressions
11 months ago

The basic functionality is in the transactions patch. Let's revisit this ticket once that lands to make sure we handle it in an aaccessible, user-friendly way, and account for all of the cases listed in the above slack conversation.

#5 @westonruter
11 months ago

See also discussion regarding a preview heartbeat to catch scenarios where a user is navigated to another URL without the Customizer preview context: https://wordpress.slack.com/archives/core-customize/p1462222433000451

#6 @westonruter
10 months ago

  • Milestone changed from 4.6 to Future Release

Punting due to transactions being too large to finish patch and test in time for 4.6.

#7 @westonruter
6 months ago

  • Description modified (diff)
  • Keywords has-patch needs-testing added
  • Milestone changed from Future Release to 4.7
  • Owner set to westonruter
  • Status changed from new to accepted

This has been implemented in the patch for #30937.

When a link is not previewable it will get a cursor:not-allowed style and wp.a11y.speak() will be called to accessibly notify that it cannot be live previewed. Likewise, when attempting to submit a form that is not live-previewable (if it uses a POST method or has an action that points off of the site) there will be be the same cursor:not-allowed style added to the form and all of its inputs with a similar non-previewable message output via wp.a11y.speak().

Please test: grunt patch:https://github.com/xwp/wordpress-develop/pull/161

Last edited 6 months ago by westonruter (previous) (diff)

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


5 months ago

#9 @celloexpressions
5 months ago

Can we do anything here for touch devices? We probably need a notice after a link is clicked or a form is submitted, which would integrate with #35210.

#10 @westonruter
5 months ago

@celloexpressions There's nothing special for touch, no. As such, mobile is out of scope for this ticket and mobile users would have no change in behavior from what is existing in core. It is not clear how a notice could be displayed. A notice for mobile users should perhaps be split off into a separate ticket.

#11 @celloexpressions
5 months ago

Touch and mobile are not the same thing :) However, I'm okay with deferring mobile to another ticket. We need to make sure we consider that cursor: not-allowed is only accessible to mouse users, and wp.a11y.speak() is only accessible to screen reader users. We'll need a solution for touch and keyboard-only users as well, but those could come later if needed.

#12 @westonruter
5 months ago

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

In 38810:

Customize: Implement customized state persistence with changesets.

Includes infrastructure developed in the Customize Snapshots feature plugin.

See https://make.wordpress.org/core/2016/10/12/customize-changesets-technical-design-decisions/

Props westonruter, valendesigns, utkarshpatel, stubgo, lgedeon, ocean90, ryankienstra, mihai2u, dlh, aaroncampbell, jonathanbardo, jorbin.
See #28721.
See #31089.
Fixes #30937.
Fixes #31517.
Fixes #30028.
Fixes #23225.
Fixes #34142.
Fixes #36485.

Note: See TracTickets for help on using tickets.