Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce viewport aspect ratio validation for URL Metrics #1494

Merged
merged 7 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions plugins/optimization-detective/class-od-url-metric.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,25 @@ private function validate_data( array $data ): void {
if ( is_wp_error( $valid ) ) {
throw new OD_Data_Validation_Exception( esc_html( $valid->get_error_message() ) );
}
$aspect_ratio = $data['viewport']['width'] / $data['viewport']['height'];
$min_aspect_ratio = od_get_minimum_viewport_aspect_ratio();
$max_aspect_ratio = od_get_maximum_viewport_aspect_ratio();
if (
$aspect_ratio < $min_aspect_ratio ||
$aspect_ratio > $max_aspect_ratio
) {
throw new OD_Data_Validation_Exception(
esc_html(
sprintf(
/* translators: 1: current aspect ratio, 2: minimum aspect ratio, 3: maximum aspect ratio */
__( 'Viewport aspect ratio (%1$s) is not in the accepted range of %2$s to %3$s.', 'optimization-detective' ),
$aspect_ratio,
$min_aspect_ratio,
$max_aspect_ratio
)
)
);
}
}

/**
Expand Down
18 changes: 18 additions & 0 deletions plugins/optimization-detective/detect.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ function getCurrentTime() {
* @param {Object} args Args.
* @param {number} args.serveTime The serve time of the page in milliseconds from PHP via `microtime( true ) * 1000`.
* @param {number} args.detectionTimeWindow The number of milliseconds between now and when the page was first generated in which detection should proceed.
* @param {number} args.minViewportAspectRatio Minimum aspect ratio allowed for the viewport.
* @param {number} args.maxViewportAspectRatio Maximum aspect ratio allowed for the viewport.
* @param {boolean} args.isDebug Whether to show debug messages.
* @param {string} args.restApiEndpoint URL for where to send the detection data.
* @param {string} args.restApiNonce Nonce for writing to the REST API.
Expand All @@ -152,6 +154,8 @@ function getCurrentTime() {
export default async function detect( {
serveTime,
detectionTimeWindow,
minViewportAspectRatio,
maxViewportAspectRatio,
isDebug,
restApiEndpoint,
restApiNonce,
Expand Down Expand Up @@ -190,6 +194,20 @@ export default async function detect( {
return;
}

// Abort if the viewport aspect ratio is not in a common range.
const aspectRatio = win.innerWidth / win.innerHeight;
if (
aspectRatio < minViewportAspectRatio ||
aspectRatio > maxViewportAspectRatio
) {
if ( isDebug ) {
warn(
`Viewport aspect ratio (${ aspectRatio }) is not in the accepted range of ${ minViewportAspectRatio } to ${ maxViewportAspectRatio }.`
);
}
return;
}

// Ensure the DOM is loaded (although it surely already is since we're executing in a module).
await new Promise( ( resolve ) => {
if ( doc.readyState !== 'loading' ) {
Expand Down
2 changes: 2 additions & 0 deletions plugins/optimization-detective/detection.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ function od_get_detection_script( string $slug, OD_URL_Metrics_Group_Collection
$detect_args = array(
'serveTime' => microtime( true ) * 1000, // In milliseconds for comparison with `Date.now()` in JavaScript.
'detectionTimeWindow' => $detection_time_window,
'minViewportAspectRatio' => od_get_minimum_viewport_aspect_ratio(),
'maxViewportAspectRatio' => od_get_maximum_viewport_aspect_ratio(),
'isDebug' => WP_DEBUG,
'restApiEndpoint' => rest_url( OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ),
'restApiNonce' => wp_create_nonce( 'wp_rest' ),
Expand Down
23 changes: 23 additions & 0 deletions plugins/optimization-detective/readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,29 @@ add_filter( 'od_url_metric_freshness_ttl', '__return_zero' );

Filters the time window between serve time and run time in which loading detection is allowed to run. This amount is the allowance between when the page was first generated (and perhaps cached) and when the detect function on the page is allowed to perform its detection logic and submit the request to store the results. This avoids situations in which there are missing URL Metrics in which case a site with page caching which also has a lot of traffic could result in a cache stampede.

**Filter:** `od_minimum_viewport_aspect_ratio` (default: 0.4)

Filters the minimum allowed viewport aspect ratio for URL metrics.

The 0.4 value is intended to accommodate the phone with the greatest known aspect
ratio at 21:9 when rotated 90 degrees to 9:21 (0.429).

**Filter:** `od_maximum_viewport_aspect_ratio` (default: 2.5)

Filters the maximum allowed viewport aspect ratio for URL metrics.

The 2.5 value is intended to accommodate the phone with the greatest known aspect
ratio at 21:9 (2.333).

During development when you have the DevTools console open, for example, the viewport aspect ratio will be wider than normal. In this case, you may want to increase the maximum aspect ratio:

`
<?php
add_filter( 'od_maximum_viewport_aspect_ratio', function () {
return 5;
} );
`

**Filter:** `od_template_output_buffer` (default: the HTML response)

Filters the template output buffer prior to sending to the client. This filter is added to implement [#43258](https://core.trac.wordpress.org/ticket/43258) in WordPress core.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,52 +117,62 @@ public static function get_post( string $slug ): ?WP_Post {
* @return OD_URL_Metric[] URL metrics.
*/
public static function get_url_metrics_from_post( WP_Post $post ): array {
$this_function = __FUNCTION__;
$trigger_warning = static function ( string $message ) use ( $this_function ): void {
wp_trigger_error( $this_function, esc_html( $message ), E_USER_WARNING );
$this_function = __METHOD__;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of __FUNCTION__ was wrong as it didn't include the class name.

$trigger_error = static function ( string $message, int $error_level = E_USER_NOTICE ) use ( $this_function ): void {
wp_trigger_error( $this_function, esc_html( $message ), $error_level );
};

$url_metrics_data = json_decode( $post->post_content, true );
if ( json_last_error() !== 0 ) {
$trigger_warning(
$trigger_error(
sprintf(
/* translators: 1: Post type slug, 2: Post ID, 3: JSON error message */
__( 'Contents of %1$s post type (ID: %2$s) not valid JSON: %3$s', 'optimization-detective' ),
self::SLUG,
$post->ID,
json_last_error_msg()
)
),
E_USER_WARNING
);
$url_metrics_data = array();
} elseif ( ! is_array( $url_metrics_data ) ) {
$trigger_warning(
$trigger_error(
sprintf(
/* translators: %s is post type slug */
__( 'Contents of %s post type was not a JSON array.', 'optimization-detective' ),
self::SLUG
)
),
E_USER_WARNING
);
$url_metrics_data = array();
}

return array_values(
array_filter(
array_map(
static function ( $url_metric_data ) use ( $trigger_warning ) {
static function ( $url_metric_data ) use ( $trigger_error ) {
if ( ! is_array( $url_metric_data ) ) {
return null;
}

try {
return new OD_URL_Metric( $url_metric_data );
} catch ( OD_Data_Validation_Exception $e ) {
$trigger_warning(
$suffix = '';
if ( isset( $url_metric_data['uuid'] ) && is_string( $url_metric_data['uuid'] ) ) {
$suffix .= sprintf( ' (URL Metric UUID: %s)', $url_metric_data['uuid'] );
}

$trigger_error(
sprintf(
/* translators: 1: Post type slug. 2: Exception message. */
__( 'Unexpected shape to JSON array in post_content of %1$s post type: %2$s', 'optimization-detective' ),
OD_URL_Metrics_Post_Type::SLUG,
$e->getMessage()
)
$e->getMessage() . $suffix
),
// This is not a warning because schema changes will happen, and so it is expected
// that this will result in existing URL metrics being invalidated.
E_USER_NOTICE
);

return null;
Expand Down
44 changes: 44 additions & 0 deletions plugins/optimization-detective/storage/data.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,50 @@ function od_verify_url_metrics_storage_nonce( string $nonce, string $slug, strin
return (bool) wp_verify_nonce( $nonce, "store_url_metrics:$slug:$url" );
}

/**
* Gets the minimum allowed viewport aspect ratio for URL metrics.
*
* @since n.e.x.t
* @access private
*
* @return float Minimum viewport aspect ratio for URL metrics.
*/
function od_get_minimum_viewport_aspect_ratio(): float {
/**
* Filters the minimum allowed viewport aspect ratio for URL metrics.
*
* The 0.4 default value is intended to accommodate the phone with the greatest known aspect
* ratio at 21:9 when rotated 90 degrees to 9:21 (0.429).
*
* @since n.e.x.t
*
* @param float $minimum_viewport_aspect_ratio Minimum viewport aspect ratio.
*/
return (float) apply_filters( 'od_minimum_viewport_aspect_ratio', 0.4 );
}

/**
* Gets the maximum allowed viewport aspect ratio for URL metrics.
*
* @since n.e.x.t
* @access private
*
* @return float Maximum viewport aspect ratio for URL metrics.
*/
function od_get_maximum_viewport_aspect_ratio(): float {
/**
* Filters the maximum allowed viewport aspect ratio for URL metrics.
*
* The 2.5 default value is intended to accommodate the phone with the greatest known aspect
* ratio at 21:9 (2.333).
*
* @since n.e.x.t
*
* @param float $maximum_viewport_aspect_ratio Maximum viewport aspect ratio.
*/
return (float) apply_filters( 'od_maximum_viewport_aspect_ratio', 2.5 );
}

/**
* Gets the breakpoint max widths to group URL metrics for various viewports.
*
Expand Down
5 changes: 3 additions & 2 deletions plugins/optimization-detective/storage/rest-api.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,13 @@ function od_handle_rest_request( WP_REST_Request $request ) {
);
} catch ( OD_Data_Validation_Exception $e ) {
return new WP_Error(
'url_metric_exception',
'rest_invalid_param',
sprintf(
/* translators: %s is exception name */
__( 'Failed to validate URL metric: %s', 'optimization-detective' ),
$e->getMessage()
)
),
array( 'status' => 400 )
);
}

Expand Down
36 changes: 36 additions & 0 deletions plugins/optimization-detective/tests/storage/test-data.php
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,42 @@ static function ( int $life, string $action ) use ( &$nonce_life_actions ): int
}
}

/**
* Test od_get_minimum_viewport_aspect_ratio().
*
* @covers ::od_get_minimum_viewport_aspect_ratio
*/
public function test_od_get_minimum_viewport_aspect_ratio(): void {
$this->assertSame( 0.4, od_get_minimum_viewport_aspect_ratio() );

add_filter(
'od_minimum_viewport_aspect_ratio',
static function () {
return '0.6';
}
);

$this->assertSame( 0.6, od_get_minimum_viewport_aspect_ratio() );
}

/**
* Test od_get_maximum_viewport_aspect_ratio().
*
* @covers ::od_get_maximum_viewport_aspect_ratio
*/
public function test_od_get_maximum_viewport_aspect_ratio(): void {
$this->assertSame( 2.5, od_get_maximum_viewport_aspect_ratio() );

add_filter(
'od_maximum_viewport_aspect_ratio',
static function () {
return 3;
}
);

$this->assertSame( 3.0, od_get_maximum_viewport_aspect_ratio() );
}

/**
* Test od_get_breakpoint_max_widths().
*
Expand Down
15 changes: 14 additions & 1 deletion plugins/optimization-detective/tests/storage/test-rest-api.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ function ( $params ) {
'depth' => 200,
),
),
'invalid_viewport_aspect_ratio' => array(
'viewport' => array(
'width' => 1024,
'height' => 12000,
),
),
'invalid_elements_type' => array(
'elements' => 'bad',
),
Expand Down Expand Up @@ -235,7 +241,14 @@ public function test_rest_request_breakpoint_not_needed_for_any_breakpoint(): vo
foreach ( $viewport_widths as $viewport_width ) {
$this->populate_url_metrics(
$sample_size,
$this->get_valid_params( array( 'viewport' => array( 'width' => $viewport_width ) ) )
$this->get_valid_params(
array(
'viewport' => array(
'width' => $viewport_width,
'height' => ceil( $viewport_width / 2 ),
),
)
)
);
}

Expand Down
Loading