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

3.18 - Front-end display - Duplicated tag added to the page #7104

Open
Miraeld opened this issue Nov 13, 2024 · 1 comment · Fixed by #7105
Open

3.18 - Front-end display - Duplicated tag added to the page #7104

Miraeld opened this issue Nov 13, 2024 · 1 comment · Fixed by #7105
Assignees
Labels
type: bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@Miraeld
Copy link
Contributor

Miraeld commented Nov 13, 2024

Describe the bug

When we are enabling the Host Google Font, we are getting a duplicated tag in the HTML.

To Reproduce

  1. Move yourself to the branch feature/7063-google-font-front-end from Closes #7063: Google font - Frontend part #7091
  2. Enable Host Google Font in WPR settings under the Media tab.
  3. Load a page with a google font,
  4. Check the HTML of the page.

For example, if on my page I get
<link href="https://fonts.googleapis.com/css?family=Roboto" rel="stylesheet">
As an output I'll get the following:

<link rel="stylesheet" href="http://localhost:8902/wp-content/cache/fonts/1/6/a/5/45cf0dd85cb9d27a7fcae31c68aee.css" data-wpr-hosted-gf-parameters="family=Roboto&amp;">
<link rel="stylesheet" href="http://localhost:8902/wp-content/cache/fonts/1/6/a/5/45cf0dd85cb9d27a7fcae31c68aee.css" data-wpr-hosted-gf-parameters="family=Roboto&amp;">

Expected behavior

We should have only one tag added.

Additional context

To get more understanding of the issue, this is happening before the Google Font Combination is happening before our new feature. This way, we don't have to duplicate the combination of google font. That's the easiest way to do and this is what we did.

However, in here:

protected function get_optimized_markup( string $url ): string {

We are adding the rel="preload" tag that is annoying us.

If the rel="preload" isn't necessary while we are hosting the font css locally, we could add a check wether the new feature is enabled or not, depending on this, we could add the preload tag or not.

@Miraeld Miraeld added the type: bug Indicates an unexpected problem or unintended behavior label Nov 13, 2024
@Miraeld Miraeld added this to the 3.18 milestone Nov 13, 2024
@Miraeld Miraeld self-assigned this Nov 13, 2024
@Miraeld
Copy link
Contributor Author

Miraeld commented Nov 13, 2024

Scope a solution

As mentioned in the Additional Context of the issue, we need to avoid adding the preload tag in here
if the newly added feature is enabled (host_fonts_locally).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant