phpbb / phpbb-translation-validator

A tool checking packages for compliance with the policies https://area51.phpbb.com/docs/dev/3.2.x/language/guidelines.html
GNU General Public License v2.0
8 stars 8 forks source link

Allow br and hr without closing /> #43

Closed Crizz0 closed 6 years ago

Crizz0 commented 6 years ago

Fixes #41

@milescellar Can you test this with your language package?

qiaeru commented 6 years ago

@Crizz0 Tested. Some strings passed, but some don't (when there are multiple <br> tags, that won't pass).

Example: https://travis-ci.org/milescellar/phpbb-language-fr/jobs/297614141

Crizz0 commented 6 years ago

Thanks for testing.

Please try again. :)

@nickvergessen What do you think?

qiaeru commented 6 years ago

@Crizz0 It looks good now! Example: https://travis-ci.org/milescellar/phpbb-language-fr/jobs/297614141

qiaeru commented 6 years ago

However, I just tried to replace all old XHTML closing tags with HTML5 ones for consistency code, but the validator seems to see additional tags when these changes are not applied to the British English language pack.

Commit: https://github.com/milescellar/phpbb-language-fr/commit/a9fe9f6e0ea0dd3a7d48762562c33bd2449a749b Failing build: https://travis-ci.org/milescellar/phpbb-language-fr/jobs/297680400

Also, there is still a missing tag error with the <img> tag, when I replace <img src="https://www.phpbb.com" /> with <img src="https://www.phpbb.com"> (which is HTML5 valid).

No more issue with other tags. 😉

But I think that's more an issue to fix with the British English language pack than the validator?

Crizz0 commented 6 years ago

Yes, it is. I will take a look.

Both versions should be allowed and shouldn't lead to a test failure.

Where do you use img?

Crizz0 commented 6 years ago

@milescellar Are you sure, that you use the last version of this PR? I got only notices, if I have converted the <br /> in 4 cases to <br>, but no FATAL errors for them. Although my english-language packages has still its <br /> there in the same place.

qiaeru commented 6 years ago

Yes I'm sure. I changed all the <br /> tags to <br> when it's possible on the language files and I'm having these fatal errors. It's because the validator just compare my language pack to the British English one and see that there are different tags (because it don't know that the <br> and <br /> tags have the same function).

Crizz0 commented 6 years ago

The thing is, I cannot reproduce this, yet. I did just not have this FATAL error their with my tests with the german language and the 3.2 en/-original one. I'll try yours.

Edit: Even with your fr/ package, I have only this errors:

 Fatal in language/fr/acp/permissions_phpbb.php:
Must not contain key: ACL_F_LIST_TOPICS

 Fatal in language/fr/cli.php:
Must not contain key: CLI_EXTENSION_NOT_ENABLEABLE

 Fatal in language/fr/common.php:
Must not contain key: PAGE_NOT_FOUND

Well, your fork is not up to date. See: https://github.com/milescellar/phpbb-translation-validator/blob/v2.0.1/src/Phpbb/TranslationValidator/Validator/LangKeyValidator.php#L735-L740

compared to my PR: https://github.com/Crizz0/phpbb-translation-validator/blob/b7bffac1d7a150d362294840d5f8c18db10bf106/src/Phpbb/TranslationValidator/Validator/LangKeyValidator.php#L791-L798

Regards

qiaeru commented 6 years ago

I just tried again with the missing code from your commit, sorry for that.

Most fatal errors have been fixed, but some are still present in 9 strings (where the content is long): https://travis-ci.org/milescellar/phpbb-language-fr/jobs/298454766

Note that I just replaced all <br /> tags to <br> on the above example, but I didn't replaced <img src="" /> to <img src="">.

You can see if I replace this tag (there is only one occurrence in HELP_BBCODE_IMAGES_BASIC_ANSWER on help/bbcode.php), there is also a fatal error: https://travis-ci.org/milescellar/phpbb-language-fr/jobs/298471882

Crizz0 commented 6 years ago

Works, but for <img ... /> is complains still for additional html with Line 755. Maybe an exception would be okay for this if-sentence.

qiaeru commented 6 years ago

I confirm, the new patch works well but there is still a fatal error for the <img> tag. https://travis-ci.org/milescellar/phpbb-language-fr/jobs/298471882

qiaeru commented 6 years ago

The latest commit (251e954) is causing a build failure.

Crizz0 commented 6 years ago

It looks like, this brackets are necessary. Because without them, it throws: PHP Parse error: syntax error, unexpected '&&' (T_BOOLEAN_AND) in /home/travis/build/phpbb/phpbb-translation-validator/src/Phpbb/TranslationValidator/Validator/LangKeyValidator.php on line 698

Crizz0 commented 6 years ago

@milescellar Any other problems with this patch?

qiaeru commented 6 years ago

There is still a fatal error concerning the <img> tag.

See: https://travis-ci.org/milescellar/phpbb-language-fr/jobs/322712993

Crizz0 commented 6 years ago

Yes, but I seems like, the validator doesn't recognizes the img without / as no-additional HTML, no matter who is missing the /> (en or your/any language to validate).

Maybe I'll push this to 1.4.1 and get 1.4.0 online for 3.2.2 translations, first.

qiaeru commented 6 years ago

Yeap, or the other way to fix it is to remove that slash in the language file itself (to keep XHTML tag specs is not a good idea). But I agree with you, let's release 1.4.0 for 3.2.2 translations, it's not a big issue. 🙂