Fix bug 1193860: escape quotes in some DTD files #326
Conversation
| @@ -78,6 +85,9 @@ def __init__(self, parser, path, source_resource=None): | |||
| self.source_resource = source_resource | |||
| self.entities = OrderedDict() # Preserve entity order. | |||
|
|
|||
| # Bug 1193860: unescape quotes in some files | |||
| self.escape_quotes_on = 'mobile/android/base' in path and parser is DTDParser | |||
There was a problem hiding this comment.
Is there any better heuristic for this? Is it bad if we escape all quotes in all DTD files?
There was a problem hiding this comment.
Yeah, that was my original idea (do it for all .dtd files), but it's sadly not possible:
https://dxr.mozilla.org/mozilla-central/source/python/compare-locales/compare_locales/checks.py#389
We have to do it the ugly way. One day we should get rid of the hardcoded path and move this to project config.
|
r+, nice work! I think you should really avoid the keying on the translation file path, and I suspect you could do simpler than a regex, but I trust your judgement on whether to actually do those things. |
|
Thanks. Turns out we don't need to escape curly quotes. I'll add tests tomorrow and then merge. |
|
Maybe I misunderstand your patch or how DTDs work, in this case just ignore this comment. But if I understand correct your patch, you always escape the quotes in the DTD files with '. I think, in DTD files, witch use a single quote to delimit the String (like search_strings.dtd), this does not work. I that case, the quote has to be replaced with something like '. Background: |
|
Good point @gion-andri! Could you please check if it looks better now? |
|
This looks way better, thanks! |
|
r+ On Thu, Jan 14, 2016 at 9:04 AM, Gion-Andri Cantieni <
Kind regards Jarek "jotes" Śmiejczak, |
Fix bug 1193860: escape quotes in some DTD files
This effectively reverts PR mozilla#326.
Some DTD files require quotes to be escaped. This is my attempt to do so.
@jotes or @Osmose r?
The curly quotes (‘ and “) don't get escaped for some reason. Any idea why? I'll add tests.