Site Health: Text adjustment (add a link and remove "we")

[birgire]

It would be helpful, I think, to have a link to the Site Health Status page within this text:

"If we see anything here that could be improved, we will let you know on the Site Health Status page."

on the /wp-admin/site-health.php?tab=debug page.

Additionally it could be adjusted further to avoid the "We" self-reference, as suggested in #46057.

Also "anything here" is too general, in my mind.

cheers

[Clorith]

Good points, the string here could be improved upon to be more useful.

I've marked it for 5.3, so we can improve on it for the next release, as we're past the string freeze for the current release cycle.

[mukesh27]

Updated text.

[mukesh27]

@birgire and @Clorith Please suggest text if needed.

[subrataemfluence]

Added suggested link against text "Site Health Status page".

[subrataemfluence]

@mukesh27 I have added the suggested link. Also a related thought (since the text link was suggested). As the page can turn into a long one when all or some sections are expanded, how about adding the same link at the bottom of the page?

[birgire]

Thanks @mukesh27 and @subrataemfluence for the patches.

In 46946.diff​ does the part:

If any improvement needed then it will show on the ...

need an "is" after the "improvement" ? (I'm not a native English speaker).

Regarding the link in In 46946-2.diff​, I would suggest using:

esc_url( admin_url( 'site-health.php' ) )

instead of

esc_url( admin_url()  . 'site-health.php' )

to e.g. make the path accessible in the admin_url filter.

Also a related thought (since the text link was suggested). As the page can turn into a long one when all or some sections are expanded, how about adding the same link at the bottom of the page?

Thanks for the suggestion on @subrataemfluence. Maybe this consideration would be better focused in a possible new ticket? I guess other admin pages will reuse this design in the future. How to help navigate long pages, in this new design, is a good question.

[subrataemfluence]

Updated patch. I also excluded the word "page" from the link as per the original ticket description.

[subrataemfluence]

Thank you @birgire. As per your advice I have created a new ticket #46956 with the following enhancement request.

Related #46956.

Thanks for the suggestion on @subrataemfluence. Maybe this consideration would be better focused in a possible new ticket? I guess other admin pages will reuse this design in the future. How to help navigate long pages, in this new design, is a good question.

[mukesh27]

Patch 46946-3.diff looks good for me

[birgire]

In 46946-3.diff​ it seems like __ and printf have to change order.

We might also add a translator comment on %s.

[leogermani]

I have updated the patches with:

• the fix @birgire mentions that __ was misplaced
• adding a translators comment
• adding the "is" to the second sentence

I also applied best practices documented at ​https://codex.wordpress.org/I18n_for_WordPress_Developers#HTML that makes use of wp_kses for extra security.

( patch created during the WCEU 2019 Dev Day :) )

[birgire]

Thank you for the updated patch @leogermani

Regarding core translations, I don't recall wp_kses used, even though it contains HTML.

This came up recently here:

https://core.trac.wordpress.org/ticket/47070#comment:43

and the patch there was committed without wp_kses.

ps: Awesome, I hope you're enjoying WCEU 2019 :-)

[leogermani]

Replying to birgire:

Regarding core translations, I don't recall wp_kses used, even though it contains HTML.

Yes, me neither. I think this is kind of super paranoic, given all translations gets reviewd and everything. And also the code gets much more ugly and complicated. But I decided to follow what is in the codex. Do you think I should take it out and submit a new patch?

Maybe Ill go to the polyglots team and see what they think :)

[leogermani]

In fact, I was wondering if instead of:

printf( __('Here is my <a href="%s">link to the admin</a>'), admin_url() );

We should have (with appropriate translators comment):

printf( __('Here is my %1$slink to the admin%2$s'), '<a href="' . admin_url() . '">', '</a>' );

This would take the burden of HTML out of the translators hand...

[foack]

Replying to leogermani:

In fact, I was wondering if instead of:

printf( __('Here is my <a href="%s">link to the admin</a>'), admin_url() );

We should have (with appropriate translators comment):

printf( __('Here is my %1$slink to the admin%2$s'), '<a href="' . admin_url() . '">', '</a>' );

This would take the burden of HTML out of the translators hand...

I like that approach. Definitely an increase in usability imho.

[leogermani]

So basically we have 3 options on how to localize a string with a HTML link in it and Im going to summarize them here in this comment to make it easier to get feedback on which is the best.

Option 1, basic

printf( __('Here is my <a href="%s">link to the admin</a>'), admin_url() ); `

Option 2, get HTML out of the translatable string

printf( __('Here is my %1$slink to the admin%2$s'), '<a href="' . admin_url() . '">', '</a>' );

Option 3, As suggested in the codex with extra security precautions (​https://codex.wordpress.org/I18n_for_WordPress_Developers#HTML)

printf(
    wp_kses(
        __('Here is my <a href="%s">link to admin</a>'),
        ['a' => ['href' => [] ] ]
    ), 
    esc_url( admin_url( 'site-health.php' ) ) );
)

In my opinion, option 3 looks good. Less chance that the HTML gets broken during the translation and, in simple cases such as this example, we could use only %s instead of %1$s, which would make it even simpler.

[desrosj]

Moving Site Health tickets into their lovely new home, the Site Health component.

[tobifjellner]

Replying to leogermani:

In fact, I was wondering if instead of:

printf( __('Here is my <a href="%s">link to the admin</a>'), admin_url() );

We should have (with appropriate translators comment):

printf( __('Here is my %1$slink to the admin%2$s'), '<a href="' . admin_url() . '">', '</a>' );

This would take the burden of HTML out of the translators hand...

Both ways are totally OK and we see both of them in various places in Core.
But for version 2, it is critically important that a good explanation is given in a comment to translators. (Personally, I prefer number 1, but again: both ways are fully ok)

[birgire]

There are both cons/pros to each approach.

The downside I see with option 2, is that the word boundary get's more blurry imho, with %1$slink.

[leogermani]

I was going to look to the source to see what is often used but @tobifjellner already did it, so great!

I had a quick chat with @nao to get an input from the translators and she also said both are fine but that maybe the first option, with the HTML inside, looks cleaner.

I will update the patch with this approach.

As for the documentation in the Codex suggesting to use wp_kses, I have an impression that this is a little too much. If we are worried that malicious HTML could be inserted by a translation, we would have to filter all strings, not only those that have a link, because the malicious code could be inserted in any of them.

I will have a look if that page was already merged to the new Developer Handbook and, if not, I will do so and suggest a change to that recommendation.

Thank you all for the inputs.

[birgire]

Thank you for the updated patch @leogermani

The core translations are considered safe (see e.g. here) so 46946-5.diff​ looks good to me.

The old Codex pages might not been updated recently.

To hopefully land this in the next milestone, it's required for tickets to have an owner, so I mark you as an owner here @leogermani, if you don't mind :-)

[SergeyBiryukov]

In 45791:

Site Health: Simplify the language in Status Health Info page introduction, add a link to Status page.

Props birgire, mukesh27, subrataemfluence, leogermani.
Fixes #46946.