Skip to content

Commit 726e112

Browse files
westonruterwestonruter
committed
Merge branch 'develop' of github.com:ampproject/amp-wp into bug/6830-page-cache-detection
* 'develop' of github.com:ampproject/amp-wp: Update comments to note where related skip link logic is located Improve nested CSS indentation Use AMP prefix for styles injected by sanitizer Remove needless escaping in DOM context Simplify obtaining main element and body element Add more test cases for skip link Normalize config.allow-plugins in composer.json Add test case for array callback with invoke_with_first_ref_arg Improve wrapped callbacks that pass reference and add tests for wrap_hook_callbacks Add test for actions with callbacks passing by reference Fix phpcs complaints Test that the before_invoke function runs Tidy phpdoc comments Account for validation-wrapped callbacks in plugin suppression Address feedback from PR and update unit test cases Fix phpstan issues Add unit test cases Add skip link in Full Site Editing themes
2 parents 844cd16 + b311b57 commit 726e112

9 files changed

+512
-50
lines changed

‎includes/class-amp-theme-support.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ public static function add_hooks() {
853853
remove_action( 'wp_head', 'print_emoji_detection_script', 7 );
854854
remove_action( 'wp_print_styles', 'print_emoji_styles' );
855855

856-
// Temporary workarounds for <https://github.com/ampproject/amp-wp/issues/6115>.
856+
// The AMP version of the skip link is implemented by AMP_Accessibility_Sanitizer::add_skip_link().
857857
remove_action( 'wp_footer', 'gutenberg_the_skip_link' );
858858
remove_action( 'wp_footer', 'the_block_template_skip_link' );
859859

‎includes/sanitizers/class-amp-accessibility-sanitizer.php

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
* @package AmpProject\AmpWP
66
*/
77

8+
use AmpProject\Dom\Element;
89
use AmpProject\Html\Attribute;
910
use AmpProject\Html\Role;
11+
use AmpProject\Html\Tag;
1012

1113
/**
1214
* Sanitizes attributes required for AMP accessibility requirements.
@@ -21,6 +23,7 @@ class AMP_Accessibility_Sanitizer extends AMP_Base_Sanitizer {
2123
*/
2224
public function sanitize() {
2325
$this->add_role_and_tabindex_to_on_tap_actors();
26+
$this->add_skip_link();
2427
}
2528

2629
/**
@@ -66,4 +69,94 @@ static function ( $predicate ) {
6669
}
6770
}
6871
}
72+
73+
/**
74+
* Add skip link markup and style.
75+
*
76+
* This is the implementation of the non-AMP logic in `the_block_template_skip_link()` which is unhooked in
77+
* `AMP_Theme_Support::add_hooks()` to prevent validation errors from being raised.
78+
*
79+
* @see AMP_Theme_Support::add_hooks()
80+
* @see the_block_template_skip_link()
81+
* @return void
82+
*/
83+
public function add_skip_link() {
84+
85+
// Early exit if not a block theme.
86+
if ( ! current_theme_supports( 'block-templates' ) ) {
87+
return;
88+
}
89+
90+
// Early exit if not a block template.
91+
global $_wp_current_template_content;
92+
if ( ! $_wp_current_template_content ) {
93+
return;
94+
}
95+
96+
$main_tag = $this->dom->getElementsByTagName( Tag::MAIN )->item( 0 );
97+
if ( ! $main_tag instanceof Element ) {
98+
return;
99+
}
100+
101+
$skip_link_target = $this->dom->getElementId( $main_tag, 'wp--skip-link--target' );
102+
103+
// Style for skip link.
104+
$style_content = '
105+
.skip-link.screen-reader-text {
106+
border: 0;
107+
clip: rect(1px,1px,1px,1px);
108+
clip-path: inset(50%);
109+
height: 1px;
110+
margin: -1px;
111+
overflow: hidden;
112+
padding: 0;
113+
position: absolute !important;
114+
width: 1px;
115+
word-wrap: normal !important;
116+
}
117+
118+
.skip-link.screen-reader-text:focus {
119+
background-color: #eee;
120+
clip: auto !important;
121+
clip-path: none;
122+
color: #444;
123+
display: block;
124+
font-size: 1em;
125+
height: auto;
126+
left: 5px;
127+
line-height: normal;
128+
padding: 15px 23px 14px;
129+
text-decoration: none;
130+
top: 5px;
131+
width: auto;
132+
z-index: 100000;
133+
}
134+
';
135+
136+
$style_node = AMP_DOM_Utils::create_node(
137+
$this->dom,
138+
Tag::STYLE,
139+
[
140+
Attribute::ID => 'amp-skip-link-styles',
141+
]
142+
);
143+
144+
$style_node->appendChild( $this->dom->createTextNode( $style_content ) );
145+
146+
// Skip link node.
147+
$skip_link = AMP_DOM_Utils::create_node(
148+
$this->dom,
149+
Tag::A,
150+
[
151+
Attribute::CLASS_ => 'skip-link screen-reader-text',
152+
Attribute::HREF => "#$skip_link_target",
153+
]
154+
);
155+
156+
$skip_link->appendChild( $this->dom->createTextNode( __( 'Skip to content', 'amp' ) ) );
157+
158+
$body = $this->dom->body;
159+
$body->insertBefore( $skip_link, $body->firstChild );
160+
$body->insertBefore( $style_node, $body->firstChild );
161+
}
69162
}

‎includes/validation/class-amp-validation-callback-wrapper.php

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,29 @@ class AMP_Validation_Callback_Wrapper implements ArrayAccess {
2020
*/
2121
protected $callback;
2222

23+
/**
24+
* Function to call before invoking the callback.
25+
*
26+
* @var callable|null
27+
*/
28+
protected $before_invoke;
29+
2330
/**
2431
* AMP_Validation_Callback_Wrapper constructor.
2532
*
26-
* @param array $callback {
33+
* @param array $callback {
2734
* The callback data.
2835
*
2936
* @type callable $function
3037
* @type int $accepted_args
3138
* @type array $source
3239
* @type array $indirect_sources
3340
* }
41+
* @param callable|null $before_invoke Additional callback to run before invoking original callback. Optional.
3442
*/
35-
public function __construct( $callback ) {
36-
$this->callback = $callback;
43+
public function __construct( $callback, $before_invoke = null ) {
44+
$this->callback = $callback;
45+
$this->before_invoke = $before_invoke;
3746
}
3847

3948
/**
@@ -50,7 +59,7 @@ public function get_callback_function() {
5059
*
5160
* @since 1.5
5261
*
53-
* @param array ...$args Arguments.
62+
* @param mixed ...$args Arguments.
5463
* @return array Preparation data.
5564
*
5665
* @global WP_Scripts|null $wp_scripts
@@ -124,6 +133,10 @@ protected function prepare( ...$args ) {
124133
* @return mixed Response.
125134
*/
126135
public function __invoke( ...$args ) {
136+
if ( $this->before_invoke ) {
137+
call_user_func( $this->before_invoke );
138+
}
139+
127140
$preparation = $this->prepare( ...$args );
128141

129142
$result = call_user_func_array(
@@ -146,12 +159,26 @@ public function __invoke( ...$args ) {
146159
* @return mixed
147160
*/
148161
public function invoke_with_first_ref_arg( &$first_arg, ...$other_args ) {
162+
if ( $this->before_invoke ) {
163+
call_user_func( $this->before_invoke );
164+
}
165+
149166
$preparation = $this->prepare( $first_arg, ...$other_args );
150167

151-
$result = $this->callback['function'](
152-
$first_arg,
153-
...array_slice( $other_args, 0, (int) $this->callback['accepted_args'] - 1 )
154-
);
168+
$function = $this->callback['function'];
169+
$other_accepted_args = array_slice( $other_args, 0, (int) $this->callback['accepted_args'] - 1 );
170+
171+
if ( $function instanceof self ) {
172+
$result = $function->invoke_with_first_ref_arg(
173+
$first_arg,
174+
...$other_accepted_args
175+
);
176+
} else {
177+
$result = $function(
178+
$first_arg,
179+
...$other_accepted_args
180+
);
181+
}
155182

156183
$this->finalize( $preparation );
157184

‎includes/validation/class-amp-validation-manager.php

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,7 +1364,7 @@ public static function wrap_block_callbacks( $args ) {
13641364
self::$original_block_render_callbacks[ $args['name'] ] = $original_function;
13651365
}
13661366

1367-
$args['render_callback'] = self::wrapped_callback(
1367+
$args['render_callback'] = new AMP_Validation_Callback_Wrapper(
13681368
[
13691369
'function' => $original_function,
13701370
'source' => $source,
@@ -1396,7 +1396,7 @@ public static function wrap_widget_callbacks() {
13961396
$accepted_args = 2; // For the $instance and $args arguments.
13971397
$callback = compact( 'function', 'accepted_args', 'source' );
13981398

1399-
$registered_widget['callback'] = self::wrapped_callback( $callback );
1399+
$registered_widget['callback'] = new AMP_Validation_Callback_Wrapper( $callback );
14001400
}
14011401
}
14021402

@@ -1461,23 +1461,23 @@ public static function wrap_hook_callbacks( $hook ) {
14611461
$source['hook'] = $hook;
14621462
$source['priority'] = $priority;
14631463
$original_function = $callback['function'];
1464-
$wrapped_callback = self::wrapped_callback(
1464+
1465+
$wrapped_callback = new AMP_Validation_Callback_Wrapper(
14651466
array_merge(
14661467
$callback,
14671468
compact( 'priority', 'source', 'indirect_sources' )
1468-
)
1469+
),
1470+
static function () use ( &$callback, $original_function ) {
1471+
// Restore the original callback function in case other plugins are introspecting filters.
1472+
// This logic runs immediately before the original function is actually invoked.
1473+
$callback['function'] = $original_function;
1474+
}
14691475
);
14701476

14711477
if ( 1 === $passed_by_ref ) {
1472-
$callback['function'] = static function( &$first, ...$other_args ) use ( &$callback, $wrapped_callback, $original_function ) {
1473-
$callback['function'] = $original_function; // Restore original.
1474-
return $wrapped_callback->invoke_with_first_ref_arg( $first, ...$other_args );
1475-
};
1478+
$callback['function'] = [ $wrapped_callback, 'invoke_with_first_ref_arg' ];
14761479
} else {
1477-
$callback['function'] = static function( ...$args ) use ( &$callback, $wrapped_callback, $original_function ) {
1478-
$callback['function'] = $original_function; // Restore original.
1479-
return $wrapped_callback( ...$args );
1480-
};
1480+
$callback['function'] = $wrapped_callback;
14811481
}
14821482
}
14831483
}
@@ -1671,6 +1671,9 @@ public static function can_output_buffer() {
16711671
* this indicates which plugin it was from.
16721672
* The call_user_func_array() logic is mainly copied from WP_Hook:apply_filters().
16731673
*
1674+
* @deprecated No longer used as of 2.2.1.
1675+
* @codeCoverageIgnore
1676+
*
16741677
* @param array $callback {
16751678
* The callback data.
16761679
*

‎src/DevTools/CallbackReflection.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace AmpProject\AmpWP\DevTools;
99

10+
use AMP_Validation_Callback_Wrapper;
1011
use AmpProject\AmpWP\Infrastructure\Service;
1112
use Exception;
1213
use ReflectionFunction;
@@ -38,6 +39,37 @@ public function __construct( FileReflection $file_reflection ) {
3839
$this->file_reflection = $file_reflection;
3940
}
4041

42+
/**
43+
* Get the underlying callback in case it was wrapped by AMP_Validation_Callback_Wrapper.
44+
*
45+
* @since 2.2.1
46+
*
47+
* @param callable $callback Callback.
48+
* @return callable Original callback.
49+
*/
50+
public function get_unwrapped_callback( $callback ) {
51+
while ( $callback ) {
52+
if ( $callback instanceof AMP_Validation_Callback_Wrapper ) {
53+
$callback = $callback->get_callback_function();
54+
} elseif (
55+
is_array( $callback )
56+
&&
57+
is_callable( $callback )
58+
&&
59+
isset( $callback[0], $callback[1] )
60+
&&
61+
$callback[0] instanceof AMP_Validation_Callback_Wrapper
62+
&&
63+
'invoke_with_first_ref_arg' === $callback[1]
64+
) {
65+
$callback = $callback[0]->get_callback_function();
66+
} else {
67+
break;
68+
}
69+
}
70+
return $callback;
71+
}
72+
4173
/**
4274
* Gets the plugin or theme of the callback, if one exists.
4375
*
@@ -54,6 +86,8 @@ public function __construct( FileReflection $file_reflection ) {
5486
* }
5587
*/
5688
public function get_source( $callback ) {
89+
$callback = $this->get_unwrapped_callback( $callback );
90+
5791
$reflection = $this->get_reflection( $callback );
5892

5993
if ( ! $reflection ) {

‎src/PluginSuppression.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,11 +367,18 @@ private function prepare_user_for_response( $username ) {
367367
*/
368368
private function suppress_hooks( $suppressed_plugins ) {
369369
global $wp_filter;
370+
/** @var WP_Hook $filter */
370371
foreach ( $wp_filter as $tag => $filter ) {
371372
foreach ( $filter->callbacks as $priority => $prioritized_callbacks ) {
372373
foreach ( $prioritized_callbacks as $callback ) {
373374
if ( $this->is_callback_plugin_suppressed( $callback['function'], $suppressed_plugins ) ) {
374-
$filter->remove_filter( $tag, $callback['function'], $priority );
375+
// Obtain the original function which is necessary to pass into remove_filter() so that the
376+
// expected key will be generated by _wp_filter_build_unique_id(). Alternatively, this could
377+
// just below do `unset( $filter->callbacks[ $priority ][ $function_key ] )` but it seems safer
378+
// to re-use all the existing logic in remove_filter().
379+
$original_function = $this->callback_reflection->get_unwrapped_callback( $callback['function'] );
380+
381+
$filter->remove_filter( $tag, $original_function, $priority );
375382
}
376383
}
377384
}

0 commit comments

Comments
 (0)