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 - New feature and SaaS part #7070

Open
piotrbak opened this issue Oct 27, 2024 · 6 comments
Open

3.18 - New feature and SaaS part #7070

piotrbak opened this issue Oct 27, 2024 · 6 comments
Labels
type: enhancement Improvements that slightly enhance existing functionality and are fast to implement type: new feature Indicates the issue is a request for new functionality
Milestone

Comments

@piotrbak
Copy link
Contributor

User Story
As a user, I’d like to apply the feature to the URLs visited by saas during RUCSS or OCI/LRC

Acceptance Criteria

  • When the visits are made by SaaS (RUCSS, OCI/ALR) we need to make sure that the fonts and CSS are downloaded and the paths are rewritten
@piotrbak piotrbak added needs: r&d Needs research and development (R&D) before a solution can be proposed and scoped. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement type: new feature Indicates the issue is a request for new functionality labels Oct 27, 2024
@piotrbak piotrbak added this to the 3.18 milestone Oct 27, 2024
@piotrbak piotrbak removed the needs: r&d Needs research and development (R&D) before a solution can be proposed and scoped. label Oct 27, 2024
@Khadreal
Copy link
Contributor

Khadreal commented Nov 5, 2024

This is dependent on the completion of #7063 and #7068

Scope a solution:

In the frontend subscriber class, we add a filter rocket_performance_hints_buffer and in the call back function we call the rewrite_fonts process.
We should check if feature is set.

if ( ! $this->options->get( 'host_google_fonts', 0 ) ) {
			return $html;
		}
//start font rewrite process

Estimation
[XS]

@Khadreal
Copy link
Contributor

Khadreal commented Nov 5, 2024

@piotrbak should this visit happen when the feature is not enabled ?

@piotrbak
Copy link
Contributor Author

piotrbak commented Nov 5, 2024

@Khadreal This issue is not about making any visits. It's just to ensure that the feature works when making a request from SaaS

@jeawhanlee
Copy link
Contributor

@Khadreal I'm not sure if we are properly covering the AC in the grooming, shouldn't we be using the rocket_performance_hints_buffer for this?

@jeawhanlee
Copy link
Contributor

I don't think we need to check the query param since rocket_performance_hints_buffer will only be triggered if those query params are set.

public function start_performance_hints_buffer() {
if ( ! isset( $_GET['wpr_imagedimensions'] ) && ! isset( $_GET['wpr_lazyrendercontent'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
return;
}
if ( ! $this->buffer_tests->can_process_any_buffer() ) {
return;
}
ob_start( [ $this, 'performance_hints_buffer' ] );
}
/**
* Update images that have no width/height with real dimensions for the SaaS
*
* @param string $buffer Page HTML content.
*
* @return string Page HTML content after update.
*/
public function performance_hints_buffer( $buffer ) {
/**
* Filters the buffer content for performance hints.
*
* @since 3.17
*
* @param $buffer Page HTML content.
*/
return wpm_apply_filters_typed( 'string', 'rocket_performance_hints_buffer', $buffer );
}

@Khadreal
Copy link
Contributor

Yes, we don't need to, forgot to remove it after the last edit. We only need to check for the feature itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Improvements that slightly enhance existing functionality and are fast to implement type: new feature Indicates the issue is a request for new functionality
Projects
None yet
Development

No branches or pull requests

3 participants