-
Notifications
You must be signed in to change notification settings - Fork 71
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
Update code for custom Safe Mode text #10489
base: develop
Are you sure you want to change the base?
Conversation
Updates to match changes in Safe Mode.
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: 0 B Total Size: 1.29 MB ℹ️ View Unchanged
|
// Always use the actual array result instead of conditionally passing function name or result | ||
$custom_content = wcpay_get_jetpack_idc_custom_content(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This had been the original approach. However, we needed to use the callback due to an i18n issue #9884
In other words, I am not sure if this is a proper fix @bindlegirl, and cc @zinigor as the author of PR 9884.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on our my debug, it looks like there is a bug around this line
https://github.com/Automattic/jetpack/blame/8a4e01ce5863f03b85eb2152f50bf7fbe632f798/projects/packages/connection/src/identity-crisis/class-ui.php#L171-L178
if ( isset( $consumer['customContent'] ) && is_callable( $consumer['customContent'] ) ) {
// ✅ The callback is called correctly in this condition.
$consumer['customContent'] = call_user_func( $consumer['customContent'] );
}
if ( isset( $_SERVER['REQUEST_URI'] ) && str_starts_with( filter_var( wp_unslash( $_SERVER['REQUEST_URI'] ) ), $consumer['admin_page'] ) && strlen( $consumer['admin_page'] ) > $consumer_url_length ) {
// ❌ Somehow, this condition is never reached; therefore, the `customContent` applied by the callback above is not passed to the output correctly.
$consumer_chosen = $consumer;
$consumer_url_length = strlen( $consumer['admin_page'] );
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what way do you experience a bug there?/ Ignore this question, I see what you mean.
To be honest, I wasn't able to reproduce the problem that #9884 was trying to solve.
A proposal for several changes that should fix the custom Safe Mode text.
Fixes #9614
Changes proposed in this Pull Request
Testing instructions
npm run changelog
to add a changelog file, choosepatch
to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge