Skip to content

Fix #7417: Preload fonts external domains exclusions#63

Merged
hanna-meda merged 5 commits intodevelopfrom
enhancement/7417-preload-font-external-domains
Jul 31, 2025
Merged

Fix #7417: Preload fonts external domains exclusions#63
hanna-meda merged 5 commits intodevelopfrom
enhancement/7417-preload-font-external-domains

Conversation

@Miraeld
Copy link
Copy Markdown
Contributor

@Miraeld Miraeld commented Jul 21, 2025

Description

Fixes #7417 from WP Rocket repo
Refer to wp-media/wp-rocket#7519

Type of change

  • Enhancement (non-breaking change which improves an existing functionality).

Detailed scenario

What was tested

Implemented test scenarios:

  1. Origin-Based External Domain Filtering

✅ External CSS links from different origins (e.g., fonts.googleapis.com) are correctly identified and processed
✅ Same-origin CSS links are properly excluded from external font processing
✅ Proper window.location.href mocking ensures accurate origin comparison in test environment
2. Exclusion-Based Configuration

✅ Links matching external_font_exclusions patterns are correctly filtered out
✅ Empty exclusion arrays default to processing all external domains
✅ Substring matching works correctly for exclusion patterns

How to test

Refer to wp-media/wp-rocket#7519

Technical description

Documentation

This pull request refactors the logic for processing external font stylesheets in the BeaconPreloadFonts class, replacing the allowlist-based approach with an exclusion-based approach. It also updates the test suite to ensure the new behavior is properly validated. The most important changes include the introduction of the external_font_exclusions configuration, enhancements to origin checks for external links, and the addition of new test cases to cover the updated functionality.

Updates to font processing logic:

  • Replaced the allowlist of external font domains with an exclusion-based approach using a new external_font_exclusions configuration. This change ensures that all external CSS links are processed unless explicitly excluded. (src/BeaconPreloadFonts.js, src/BeaconPreloadFonts.jsL261-R280)

Enhancements to origin checks:

  • Added logic to verify that only external domains (different origins) are processed, improving robustness and security. This includes handling potential errors when parsing URLs. (src/BeaconPreloadFonts.js, src/BeaconPreloadFonts.jsL261-R280)

Test suite updates:

  • Introduced the external_font_exclusions property in the test configuration to validate the exclusion-based filtering mechanism. (test/BeaconPreloadFonts.test.js, test/BeaconPreloadFonts.test.jsL37-R38)
  • Added tests to ensure links are excluded based on the external_font_exclusions configuration and that only links from different origins are processed. (test/BeaconPreloadFonts.test.js, test/BeaconPreloadFonts.test.jsR1232-R1267)
  • Mocked window.location and the URL constructor in tests to simulate origin checks and ensure accurate comparisons. (test/BeaconPreloadFonts.test.js, [1] [2]

New dependencies

None

Risks

None

Mandatory Checklist

Code validation

  • I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I implemented built-in tests to cover the new/changed code.

Code style

  • I wrote a self-explanatory code about what it does.
  • I protected entry points against unexpected inputs.
  • I did not introduce unnecessary complexity.
  • Output messages (errors, notices, logs) are explicit enough for users to understand the issue and are actionnable.

@Miraeld Miraeld requested a review from a team July 21, 2025 06:02
@Miraeld Miraeld self-assigned this Jul 21, 2025
Copilot AI review requested due to automatic review settings July 21, 2025 06:02
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Jul 21, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.05% (target: -1.00%) 89.47% (target: 50.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (3be7674) 1905 1582 83.04%
Head commit (ba38b29) 1911 (+6) 1586 (+4) 82.99% (-0.05%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#63) 19 17 89.47%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@Miraeld Miraeld linked an issue Jul 21, 2025 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the external font processing logic in the BeaconPreloadFonts class from an allowlist-based approach to an exclusion-based approach, addressing issue #7417. The change enables processing of all external CSS links from different origins unless they are explicitly excluded through the new external_font_exclusions configuration.

  • Replaced hardcoded allowlist of font providers with configurable exclusion-based filtering
  • Added origin validation to ensure only external domains are processed
  • Enhanced test coverage with origin mocking and exclusion-based test scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/BeaconPreloadFonts.js Replaced allowlist filtering with exclusion-based logic and added origin validation for external CSS links
test/BeaconPreloadFonts.test.js Added external_font_exclusions config property and comprehensive tests for origin filtering and exclusion behavior

Comment thread src/BeaconPreloadFonts.js Outdated
Comment thread src/BeaconPreloadFonts.js Outdated
Comment thread src/BeaconPreloadFonts.js Outdated
@hanna-meda hanna-meda merged commit 2e2b9e8 into develop Jul 31, 2025
6 checks passed
@hanna-meda hanna-meda deleted the enhancement/7417-preload-font-external-domains branch July 31, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3.19: Preload fonts external domains

4 participants