locale_string_is_safe() is used to check that a string is safe to be added or imported as a translation. It check if html tags contained in the string are white listed or not. It uses Xss::filter which returns well-formed HTML entities. Due to this HTML correction, non-well formed html tags are refused by locale_string_is_safe(). If a translated string contains, for example <br/>, the string is rejected. While strings with <br /> are accepted.
Make locale_string_is_safe() ignore white space in html tags.
t.b.d.
none
none
none
| Comment | File | Size | Author |
|---|---|---|---|
| #29 | 953582-29-test-only.patch | 1.33 KB | acbramley |
Comments
Comment #1
gábor hojtsyhttp://api.drupal.org/api/function/locale_string_is_safe/6 is the function used to check these strings. br is an allowed tag, so not sure why this would be a problem. Can you post the whole string? Moving to l10n_client.
Comment #2
gábor hojtsyDoh, you did provide the link to the string. It is:
So I'm thinking it might be that a space is missing before the slash. However, http://api.drupal.org/api/function/_filter_xss_split/6 does seem to allow not having a space there. Interesting. Can you try with a space before the slash?
Comment #3
palik commentedWell, i played with this and it seems that "<br/>" is not always treated the same way,
1) when i set l10nclient to send strings to LDO it goes to l10n server properly, but in locale database on my site it is rejected (not saved);
2) on LDO, after i accept string with <br/> and export .po file and try to import it to my site, I get "One translation string was skipped because it contains disallowed HTML." error.
3) If i then change from <br/> to <br /> in .po file everything goes smoothly and strong gets properly saved into db.
sorry form my english i tried to write this as clearly as i could :)
PS. we (polish team) have more than 94% of D7 translated :)
Comment #4
gábor hojtsyWell, then looks like we should fix the XSS checking in core to allow for the space to be omitted (as it is possible to be omitted as per the standard). Moving to the core queue then.
Comment #5
meba commentedI somehow agree. I also had issues with other tags not allowed (like div). Are we able to dig up a specific reason why we introduced this test?
Comment #6
gábor hojtsy@meba: In locale? If there is no XSS check, translators can introduce XSS on your site via translations (which are often treated as trusted data and sometimes contain HTML markup themselves).
Comment #7
chx commentedSo what happens is that filter_xss converts <br/> to <br /> and so the string does not match up. You want to mess with the "Is there a closing XHTML slash at the end of the attributes?" section. Mess with it at your own peril, though.
Comment #8
spiffl commentedThe attached patch-file modifies the function locale_string_is_safe() by
first comparing as before the patch, i.e. the resulting strings must be equal after filter_xss.
If this fails, both strings are run through a remove-whitespace-regex and are then compared again.
Comment #9
thedavidmeister commentedThe behaviour appears to be identical in d8, so bumping version number.
This type of patch will definitely need tests before it could be considered for production.
Comment #10
sutharsan commentedComment #11
sutharsan commentedThese are tests that prove that the problem still exists in D8.
This patch does NOT include the re-roll of #8.
Comment #12
babruix commentedComment #13
sutharsan commentedThis patch adds the reroll of #8.
Comment #15
sutharsan commentedFixing tests.
I matched the tests with the code. But not 100% sure if we should cater for tabs and spaces, or only spaces.
Comment #16
sutharsan commentedUpdated issue summary.
Comment #17
sutharsan commentedComment #29
acbramley commentedRe-wrote the Xss::filter test coverage and added locale_string_is_safe coverage. Hopefully this will demonstrate where the issue lies in that Xss::filter returns markup with the whitespace added and therefore the comparison fails.
Comment #30
acbramley commentedComment #32
acbramley commented