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

Invalid HTML in CheckPHPVersion admin_notice #260

Closed
TeemuSuoranta opened this issue Jul 5, 2019 · 11 comments

Comments

@TeemuSuoranta
Copy link
Contributor

commented Jul 5, 2019

Version: 1.9.9

The admin_notice for CheckPHPVersion returns invalid HTML:

<div class="notice notice-info">
      <p>
      PHP-versio 7.3 on saatavilla mutta ei käytössä. Kehittäjät haluavat todennäköisesti <a href="tools.php?page=updates_page">päivittää uusimpaan</a> PHP-versioon paremman suorituskyvyn ja uusien ominaisuuksien takia. Lue lisää <a target="_blank" href="https://help.seravo.com/fi/knowledgebase/13/docs/1      </p>
      </div>

This doesn't completely break anything as browsers will still render it to some degree. Visually the admin_notice is just huge (on Mac Chrome) when the markup is broken like this.

Not sure why it's broken. Probably the Finnish translation is somehow broken and it might be a good idea to break that string down to multiple strings and avoid " marks in translation files.

@TeemuSuoranta TeemuSuoranta added the bug label Jul 5, 2019

@TeemuSuoranta TeemuSuoranta changed the title Invalid HTML in CheckPHPVersion admin_notive Invalid HTML in CheckPHPVersion admin_notice Jul 5, 2019

@JoosuaKoskinen

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

Couldn't reproduce this in 1.9.10, nor in 1.9.9. I agree that the paragraph is a bit long but I see no reason for it to cut in the middle of the link.

In the snippet it cuts at 299th character. Could it maybe be something site specific?

@TeemuSuoranta

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

I've seen it on multiple production and local sites.

For example on setup:
PHP v7.2.16
WP 5.2.2
Seravo plugin 1.9.9.

Doesn't cause anything to PHP error log but the markup is similarly broken. If I change my language to English, markup won't break but with Finnish it does.

If you cannot reproduce the problem I can give you a few sites that affected via email.

@JoosuaKoskinen

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

Tried with exactly the same versions, still works fine:

<div class="notice notice-info">
      <p>
      PHP-versio 7.3 on saatavilla mutta ei käytössä. Kehittäjät haluavat todennäköisesti <a href="tools.php?page=updates_page">päivittää uusimpaan</a> PHP-versioon paremman suorituskyvyn ja uusien ominaisuuksien takia. Lue lisää <a target="_blank" href="https://help.seravo.com/fi/knowledgebase/13/docs/107-uusimpaan-php-versioon-siirtyminen">PHP-versiopäivityksistä</a>.      </p>
      </div>

Example sites would be helpful. Could you please email a couple to wordpress@seravo.fi or joosua@seravo.fi.

@TeemuSuoranta

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

Sure thing. I sent you a couple.

@JoosuaKoskinen

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

Seems like an issue with a common plugin between the given sites. Sent more info via email because of a site specific problem.

@TeemuSuoranta

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

So for the record, the affected sites seem to have mu-plugin Dynamic MO Loader (https://github.com/aucor/dynamic-mo-loader) in common that for whatever reason breaks this one string from .mo file. I have never seen dynamic-mo-loader do this kind of thing so I will look into it.

Dynamic MO Loader changes the native way that WordPress handles translations by caching them and getting them dynamically. The plugin has 16k downloads in packagist and has been mentioned by Otto in a past optimization presentation (https://www.slideshare.net/ottokekalainen/improving-wordpress-performance-with-xdebug-and-php-profiling)

I'll gladly take any help to track down the bug in Dynamic MO Loader but I understand that we are out of scope here. I'll use the .po/.mo from this plugin as reference and try to find the issue. Are you using the wp-pomo-compile (https://seravo.com/docs/get-started/available-commands/) to generate these files?

@JoosuaKoskinen

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

It definitely would be good to track the issue down. Please tell us if there's something that should be fixed on Seravo Plugin side.

I think we are using wp-pomo-compile, yes. Or msgfmt which it's a wrapper for.

@ottok

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

The .mo files in the Seravo Plugin are generated by Poedit when the person updating the translation is saving the translation.

@ottok

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

Note that I updated the translations a week ago, so updating the plugin to latest development version with wp-seravo-plugin --dev might fix the issue.

@TeemuSuoranta

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

There's been progress in this issue from our part. Firstly, yes, this is a bug in Dynamic MO Loader and not in Seravo Plugin.

Now, the reason why this bug exists is most strange. Basically Dynamic MO Loader checks if the string has 0 in it and if it has, the string will be concatenated to that point.

Timeline:

We are still testing this at the moment but will probably make a release soon that will fix this. I find it really strange that this bug has not been reported before.

@JoosuaKoskinen

This comment has been minimized.

Copy link
Member

commented Aug 17, 2019

Great that this got figured out!

Closing this now as no need to do anything with Seravo Plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.