Skip to content

Commit

Permalink
Merge pull request #1839 from WordPress/update/viewport-handling
Browse files Browse the repository at this point in the history 
Change minimum viewport width to be exclusive whereas the maximum width remains inclusive
  • Loading branch information
@westonruter
westonruter authored Feb 3, 2025
2 parents d94977f + b4ea6bd commit 5a1e472
Show file tree
Hide file tree
Showing 24 changed files with 264 additions and 219 deletions.
33 changes: 19 additions & 14 deletions plugins/optimization-detective/class-od-html-tag-processor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
* Open stack tags.
*
* @since 0.4.0
* @var string[]
* @var non-empty-string[]
*/
private $open_stack_tags = array();

Expand All @@ -160,15 +160,15 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
* are children of the `BODY` tag. This is used in {@see self::get_xpath()}.
*
* @since 1.0.0
* @var array<array<string, string>>
* @var array<array<non-empty-string, string>>
*/
private $open_stack_attributes = array();

/**
* Open stack indices.
*
* @since 0.4.0
* @var int[]
* @var non-negative-int[]
*/
private $open_stack_indices = array();

Expand All @@ -181,7 +181,7 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
* populated back into `$this->open_stack_tags` and `$this->open_stack_indices`.
*
* @since 0.4.0
* @var array<string, array{tags: string[], attributes: array<array<string, string>>, indices: int[]}>
* @var array<string, array{tags: non-empty-string[], attributes: array<array<non-empty-string, string>>, indices: non-negative-int[]}>
*/
private $bookmarked_open_stacks = array();

Expand Down Expand Up @@ -221,7 +221,7 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
* Mapping of bookmark name to a list of HTML strings which will be inserted at the time get_updated_html() is called.
*
* @since 0.4.0
* @var array<string, string[]>
* @var array<non-empty-string, string[]>
*/
private $buffered_text_replacements = array();

Expand All @@ -238,7 +238,7 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
* Count for the number of times that the cursor was moved.
*
* @since 0.6.0
* @var int
* @var non-negative-int
* @see self::next_token()
* @see self::seek()
*/
Expand Down Expand Up @@ -292,7 +292,7 @@ public function next_open_tag(): bool {
* @see WP_HTML_Processor::expects_closer()
* @since 0.4.0
*
* @param string|null $tag_name Tag name, if not provided then the current tag is used. Optional.
* @param non-empty-string|null $tag_name Tag name, if not provided then the current tag is used. Optional.
* @return bool Whether to expect a closer for the tag.
*/
public function expects_closer( ?string $tag_name = null ): bool {
Expand Down Expand Up @@ -336,6 +336,11 @@ public function next_token(): bool {
if ( null === $tag_name || $this->get_token_type() !== '#tag' ) {
return true;
}
/**
* Tag name.
*
* @var non-empty-string $tag_name
*/

if ( $this->previous_tag_without_closer ) {
array_pop( $this->open_stack_tags );
Expand Down Expand Up @@ -422,7 +427,7 @@ public function next_token(): bool {
* @see self::next_token()
* @see self::seek()
*
* @return int Count of times the cursor has moved.
* @return non-negative-int Count of times the cursor has moved.
*/
public function get_cursor_move_count(): int {
return $this->cursor_move_count;
Expand Down Expand Up @@ -458,8 +463,8 @@ public function set_attribute( $name, $value ): bool { // phpcs:ignore SlevomatC
*
* @since 0.4.0
*
* @param string $name Meta attribute name.
* @param string|true $value Value.
* @param non-empty-string $name Meta attribute name.
* @param string|true $value Value.
* @return bool Whether an attribute was set.
*/
public function set_meta_attribute( string $name, $value ): bool {
Expand Down Expand Up @@ -489,7 +494,7 @@ public function remove_attribute( $name ): bool { // phpcs:ignore SlevomatCoding
* @since 0.4.0
* @see WP_HTML_Processor::get_current_depth()
*
* @return int Nesting-depth of current location in the document.
* @return non-negative-int Nesting-depth of current location in the document.
*/
public function get_current_depth(): int {
return count( $this->open_stack_tags );
Expand Down Expand Up @@ -565,7 +570,7 @@ public function release_bookmark( $name ): bool {
* @since 0.4.0
* @since 0.9.0 Renamed from get_breadcrumbs() to get_indexed_breadcrumbs().
*
* @return Generator<array{string, int, array<string, string>}> Breadcrumb.
* @return Generator<array{non-empty-string, non-negative-int, array<non-empty-string, string>}> Breadcrumb.
*/
private function get_indexed_breadcrumbs(): Generator {
foreach ( $this->open_stack_tags as $i => $breadcrumb_tag_name ) {
Expand All @@ -584,7 +589,7 @@ private function get_indexed_breadcrumbs(): Generator {
*
* @since 1.0.0
*
* @return array<string, string> Disambiguating attributes.
* @return array<non-empty-string, string> Disambiguating attributes.
*/
private function get_disambiguating_attributes(): array {
$attributes = array();
Expand Down Expand Up @@ -617,7 +622,7 @@ private function get_disambiguating_attributes(): array {
* @since 0.9.0
* @see WP_HTML_Processor::get_breadcrumbs()
*
* @return string[] Array of tag names representing path to matched node.
* @return non-empty-string[] Array of tag names representing path to matched node.
*/
public function get_breadcrumbs(): array {
return $this->open_stack_tags;
Expand Down
14 changes: 7 additions & 7 deletions plugins/optimization-detective/class-od-link-collection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* @phpstan-type Link array{
* attributes: LinkAttributes,
* minimum_viewport_width: int<0, max>|null,
* maximum_viewport_width: positive-int|null
* maximum_viewport_width: int<1, max>|null
* }
*
* @phpstan-type LinkAttributes array{
Expand Down Expand Up @@ -53,9 +53,9 @@ final class OD_Link_Collection implements Countable {
*
* @phpstan-param LinkAttributes $attributes
*
* @param array $attributes Attributes.
* @param int<0, max>|null $minimum_viewport_width Minimum width or null if not bounded or relevant.
* @param positive-int|null $maximum_viewport_width Maximum width or null if not bounded (i.e. infinity) or relevant.
* @param array $attributes Attributes.
* @param int<0, max>|null $minimum_viewport_width Minimum width (exclusive) or null if not bounded or relevant.
* @param int<1, max>|null $maximum_viewport_width Maximum width (inclusive) or null if not bounded (i.e. infinity) or relevant.
*
* @throws InvalidArgumentException When invalid arguments are provided.
*/
Expand Down Expand Up @@ -194,9 +194,9 @@ static function ( array $carry, array $link ): array {
&&
is_int( $last_link['maximum_viewport_width'] )
&&
$last_link['maximum_viewport_width'] + 1 === $link['minimum_viewport_width']
$last_link['maximum_viewport_width'] === $link['minimum_viewport_width']
) {
$last_link['maximum_viewport_width'] = max( $last_link['maximum_viewport_width'], $link['maximum_viewport_width'] );
$last_link['maximum_viewport_width'] = null === $link['maximum_viewport_width'] ? null : max( $last_link['maximum_viewport_width'], $link['maximum_viewport_width'] );

// Update the last link with the new maximum viewport width.
$carry[ count( $carry ) - 1 ] = $last_link;
Expand Down Expand Up @@ -249,7 +249,7 @@ public function get_html(): string {
/**
* Constructs the Link HTTP response header.
*
* @return string|null Link HTTP response header, or null if there are none.
* @return non-empty-string|null Link HTTP response header, or null if there are none.
*/
public function get_response_header(): ?string {
$link_headers = array();
Expand Down
12 changes: 6 additions & 6 deletions plugins/optimization-detective/class-od-tag-visitor-registry.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class OD_Tag_Visitor_Registry implements Countable, IteratorAggregate {
/**
* Visitors.
*
* @var array<string, TagVisitorCallback>
* @var array<non-empty-string, TagVisitorCallback>
*/
private $visitors = array();

Expand All @@ -36,8 +36,8 @@ final class OD_Tag_Visitor_Registry implements Countable, IteratorAggregate {
*
* @phpstan-param TagVisitorCallback $tag_visitor_callback
*
* @param string $id Identifier for the tag visitor.
* @param callable $tag_visitor_callback Tag visitor callback.
* @param non-empty-string $id Identifier for the tag visitor.
* @param callable $tag_visitor_callback Tag visitor callback.
*/
public function register( string $id, callable $tag_visitor_callback ): void {
$this->visitors[ $id ] = $tag_visitor_callback;
Expand All @@ -46,7 +46,7 @@ public function register( string $id, callable $tag_visitor_callback ): void {
/**
* Determines if a visitor has been registered.
*
* @param string $id Identifier for the tag visitor.
* @param non-empty-string $id Identifier for the tag visitor.
* @return bool Whether registered.
*/
public function is_registered( string $id ): bool {
Expand All @@ -56,7 +56,7 @@ public function is_registered( string $id ): bool {
/**
* Gets a registered visitor.
*
* @param string $id Identifier for the tag visitor.
* @param non-empty-string $id Identifier for the tag visitor.
* @return TagVisitorCallback|null Whether registered.
*/
public function get_registered( string $id ): ?callable {
Expand All @@ -69,7 +69,7 @@ public function get_registered( string $id ): ?callable {
/**
* Unregisters a tag visitor.
*
* @param string $id Identifier for the tag visitor.
* @param non-empty-string $id Identifier for the tag visitor.
* @return bool Whether a tag visitor was unregistered.
*/
public function unregister( string $id ): bool {
Expand Down
54 changes: 26 additions & 28 deletions plugins/optimization-detective/class-od-url-metric-group-collection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,26 +47,21 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega
/**
* Breakpoints in max widths.
*
* Valid values are from 1 to PHP_INT_MAX - 1. This is because:
*
* 1. It doesn't make sense for there to be a viewport width of zero, so the first breakpoint (max width) must be at least 1.
* 2. After the last breakpoint, the final breakpoint group is set to be spanning one plus the last breakpoint max width up
* until PHP_INT_MAX. So a breakpoint cannot be PHP_INT_MAX because then the minimum viewport width for the final group
* would end up being larger than PHP_INT_MAX.
* A breakpoint must be greater than zero because a viewport group's maximum viewport width has a minimum (inclusive)
* value of 1, and the breakpoints are used as the maximum viewport widths for the viewport groups, with the addition of
* a final viewport group which has a maximum viewport width of infinity.
*
* This array may be empty in which case there are no responsive breakpoints and all URL Metrics are collected in a
* single group.
*
* @var int[]
* @phpstan-var positive-int[]
* @var positive-int[]
*/
private $breakpoints;

/**
* Sample size for URL Metrics for a given breakpoint.
*
* @var int
* @phpstan-var positive-int
* @var int<1, max>
*/
private $sample_size;

Expand All @@ -75,8 +70,7 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega
*
* A freshness age of zero means a URL Metric will always be considered stale.
*
* @var int
* @phpstan-var 0|positive-int
* @var int<0, max>
*/
private $freshness_ttl;

Expand All @@ -91,7 +85,7 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega
* get_groups_by_lcp_element?: array<string, OD_URL_Metric_Group[]>,
* get_common_lcp_element?: OD_Element|null,
* get_all_element_max_intersection_ratios?: array<string, float>,
* get_xpath_elements_map?: array<string, non-empty-array<int, OD_Element>>,
* get_xpath_elements_map?: array<string, non-empty-array<non-negative-int, OD_Element>>,
* get_all_elements_positioned_in_any_initial_viewport?: array<string, bool>,
* }
*/
Expand All @@ -102,6 +96,10 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega
*
* @throws InvalidArgumentException When an invalid argument is supplied.
*
* @phpstan-param positive-int[] $breakpoints
* @phpstan-param int<1, max> $sample_size
* @phpstan-param int<0, max> $freshness_ttl
*
* @param OD_URL_Metric[] $url_metrics URL Metrics.
* @param non-empty-string $current_etag The current ETag.
* @param int[] $breakpoints Breakpoints in max widths.
Expand All @@ -127,13 +125,13 @@ public function __construct( array $url_metrics, string $current_etag, array $br
sort( $breakpoints );
$breakpoints = array_values( array_unique( $breakpoints, SORT_NUMERIC ) );
foreach ( $breakpoints as $breakpoint ) {
if ( ! is_int( $breakpoint ) || $breakpoint < 1 || PHP_INT_MAX === $breakpoint ) {
if ( ! is_int( $breakpoint ) || $breakpoint < 1 ) {
throw new InvalidArgumentException(
esc_html(
sprintf(
/* translators: %d is the invalid breakpoint */
__(
'Each of the breakpoints must be greater than zero and less than PHP_INT_MAX, but encountered: %d',
'Each of the breakpoints must be greater than zero, but encountered: %d',
'optimization-detective'
),
$breakpoint
Expand Down Expand Up @@ -215,7 +213,7 @@ public function get_first_group(): OD_URL_Metric_Group {
*
* This group normally represents viewports for desktop devices. This group always has a minimum viewport width
* defined as one greater than the largest breakpoint returned by {@see od_get_breakpoint_max_widths()}.
* The maximum viewport is always `PHP_INT_MAX`, or in other words it is unbounded.
* The maximum viewport width of this group is always `null`, or in other words it is unbounded.
*
* @since 0.7.0
*
Expand Down Expand Up @@ -244,13 +242,13 @@ public function clear_cache(): void {
* @return OD_URL_Metric_Group[] Groups.
*/
private function create_groups(): array {
$groups = array();
$min_width = 0;
foreach ( $this->breakpoints as $max_width ) {
$groups[] = new OD_URL_Metric_Group( array(), $min_width, $max_width, $this->sample_size, $this->freshness_ttl, $this );
$min_width = $max_width + 1;
$groups = array();
$min_width_exclusive = 0;
foreach ( $this->breakpoints as $max_width_inclusive ) {
$groups[] = new OD_URL_Metric_Group( array(), $min_width_exclusive, $max_width_inclusive, $this->sample_size, $this->freshness_ttl, $this );
$min_width_exclusive = $max_width_inclusive;
}
$groups[] = new OD_URL_Metric_Group( array(), $min_width, PHP_INT_MAX, $this->sample_size, $this->freshness_ttl, $this );
$groups[] = new OD_URL_Metric_Group( array(), $min_width_exclusive, null, $this->sample_size, $this->freshness_ttl, $this );
return $groups;
}

Expand All @@ -272,7 +270,7 @@ public function add_url_metric( OD_URL_Metric $new_url_metric ): void {
}
}
// @codeCoverageIgnoreStart
// In practice this exception should never get thrown because create_groups() creates groups from a minimum of 0 to a maximum of PHP_INT_MAX.
// In practice this exception should never get thrown because create_groups() creates groups from a minimum of 0 to an unbounded maximum.
throw new InvalidArgumentException(
esc_html__( 'No group available to add URL Metric to.', 'optimization-detective' )
);
Expand All @@ -285,7 +283,7 @@ public function add_url_metric( OD_URL_Metric $new_url_metric ): void {
* @since 0.1.0
* @throws InvalidArgumentException When there is no group for the provided viewport width. This would only happen if a negative width is provided.
*
* @param int $viewport_width Viewport width.
* @param positive-int $viewport_width Viewport width.
* @return OD_URL_Metric_Group URL Metric group for the viewport width.
*/
public function get_group_for_viewport_width( int $viewport_width ): OD_URL_Metric_Group {
Expand All @@ -300,7 +298,7 @@ public function get_group_for_viewport_width( int $viewport_width ): OD_URL_Metr
}
}
// @codeCoverageIgnoreStart
// In practice this exception should never get thrown because create_groups() creates groups from a minimum of 0 to a maximum of PHP_INT_MAX.
// In practice this exception should never get thrown because create_groups() creates groups from a minimum of 0 to an unbounded maximum.
throw new InvalidArgumentException(
esc_html(
sprintf(
Expand Down Expand Up @@ -495,7 +493,7 @@ public function get_common_lcp_element(): ?OD_Element {
*
* @since 0.7.0
*
* @return array<string, non-empty-array<int, OD_Element>> Keys are XPaths and values are the element instances.
* @return array<string, non-empty-array<non-negative-int, OD_Element>> Keys are XPaths and values are the element instances.
*/
public function get_xpath_elements_map(): array {
if ( array_key_exists( __FUNCTION__, $this->result_cache ) ) {
Expand Down Expand Up @@ -667,8 +665,8 @@ public function count(): int {
* every_group_populated: bool,
* groups: array<int, array{
* lcp_element: ?OD_Element,
* minimum_viewport_width: 0|positive-int,
* maximum_viewport_width: positive-int,
* minimum_viewport_width: int<0, max>,
* maximum_viewport_width: int<1, max>|null,
* complete: bool,
* url_metrics: OD_URL_Metric[]
* }>
Expand Down
Loading

0 comments on commit 5a1e472

Please sign in to comment.