Skip to content

Fix warnings on HTML elements in placeholders #4529

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.

Sign up for GitHub

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

Merged
flodolo merged 3 commits into from
Sep 15, 2026
Merged

Fix warnings on HTML elements in placeholders #4529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6923fcb
Fix warnings on HTML elements in placeholders
flodolo Sep 15, 2026
4255576
Restore optimization (address comment)
flodolo Sep 15, 2026
a513c00
Fix linter
flodolo Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion pontoon/checks/libraries/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ def require_placeholders_match(
if src:
for pattern in get_patterns(src):
preview = ""
ph_spans: list[tuple[int, int, str]] = []
for el in pattern:
if isinstance(el, str):
if "%" in el:
Expand All @@ -272,17 +273,31 @@ def require_placeholders_match(
and el.function in (None, "html")
):
src_ph_strings.add(ps)
required_ph.add(ps)
ph_spans.append((len(preview), len(preview) + len(ps), ps))
preview += ps
enclosed_spans: set[tuple[int, int, str]] = set()
elements: Counter[str] = Counter()
src_mismatched = mismatched_tags(preview)
# Put tags back together when placeholders split them into parts.
for pm in ph_re.finditer(preview):
if pm[0].startswith("<") and (
is_element(pm[0], preview) or pm.span() in src_mismatched
):
src_ph_strings.add(pm[0])
required_ph.add(pm[0])
elements[pm[0]] += 1
enclosed_spans.update(
(start, end, ps)
for start, end, ps in ph_spans
if pm.start() <= start
and end <= pm.end()
and pm.span() != (start, end)
)
required_ph.update(
ps
for start, end, ps in ph_spans
if (start, end, ps) not in enclosed_spans
)
variant_counts = count_unnumbered_placeholders(preview)
src_max_counts |= variant_counts
src_min_counts = (
Expand Down
163 changes: 149 additions & 14 deletions pontoon/checks/tests/test_custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,32 @@ def test_android_changed_placeholder():
}


def test_android_placeholder_in_element():
"""Source XML: Read the &lt;a href="%1$s"&gt;policy&lt;/a&gt;"""
original = 'Read the <a href="{$arg1 :string @source=|%1$s|}">policy{|</a>| :html}'
entity = mock_entity("android", string=original)
assert run_custom_checks(entity, original) == {}


def test_android_changed_placeholder_in_element():
"""Source XML: Read the &lt;a href="%1$s"&gt;policy&lt;/a&gt;

Translation XML: Leggi la &lt;a href="https://example.com"&gt;policy&lt;/a&gt;
"""
original = 'Read the <a href="{$arg1 :string @source=|%1$s|}">policy{|</a>| :html}'
translation = 'Leggi la <a href="https://example.com">policy{|</a>| :html}'
entity = mock_entity("android", string=original)
assert run_custom_checks(entity, translation) == {
"pErrors": ['Element <a href="https://example.com"> not found in reference'],
"pWarnings": ['Element <a href="%1$s"> not found in translation'],
}


def test_android_changed_placeholder_in_markup():
"""Source XML: Read the <a href="%1$s">policy</a>

Translation XML: Leggi la <a href="https://example.com">policy</a>
"""
original = "Read the {#a href=|%1$s|}policy{/a}"
translation = "Leggi la {#a href=|https://example.com|}policy{/a}"
entity = mock_entity("android", string=original)
Expand All @@ -302,11 +327,19 @@ def test_android_changed_placeholder_in_element():
],
)
def test_android_unnumbered_placeholder_in_markup_attribute(original):
"""Source XML: Hi %s <a href="%s">link</a>

Source XML: <a href="%s">%s</a>
"""
entity = mock_entity("android", string=original)
assert run_custom_checks(entity, original) == {}


def test_android_extra_unnumbered_placeholder_with_markup_attribute():
"""Source XML: Hi %s <a href="%s">link</a>

Translation XML: Hi %s %s <a href="%s">link</a>
"""
original = "Hi {$arg :string @source=|%s|} {#a href=|%s|}link{/a}"
translation = (
"Hi {$arg :string @source=|%s|} {$arg2 :string @source=|%s|} "
Expand All @@ -321,6 +354,7 @@ def test_android_extra_unnumbered_placeholder_with_markup_attribute():


def test_android_paired_element_in_text():
"""Source XML: Read the <b>policy</b>"""
original = "Read the {#b}policy{/b}"
entity = mock_entity("android", string=original)
assert run_custom_checks(entity, "Leggi la {#b}policy{/b}") == {}
Expand Down Expand Up @@ -382,7 +416,10 @@ def test_android_escaped_element_with_attributes():


def test_android_mismatched_elements_in_text():
"""Tags that don't pair up are markup, whatever they're named."""
"""Tags that don't pair up are markup, whatever they're named.

Translation XML: &lt;foo&gt;testo&lt;/bar&gt;
"""
entity = mock_entity("android", string="text")
checks = run_custom_checks(entity, "{|<foo>| :html}testo{|</bar>| :html}")
assert sorted(checks["pErrors"]) == [
Expand All @@ -407,6 +444,9 @@ def test_xcode_literal_angle_brackets():


def test_android_protections_with_shared_substring():
"""Source XML:
Hi <xliff:g id="a">Name</xliff:g> and <xliff:g id="b">FullName</xliff:g>
"""
original = (
"Hi {$a :xliff:g id=a @translate=no @source=Name} "
"and {$b :xliff:g id=b @translate=no @source=FullName}"
Expand All @@ -421,12 +461,17 @@ def test_android_protections_with_shared_substring():


def test_android_multi_digit_placeholder():
"""Source XML: Hi %10$s and %2$s"""
original = "Hi {$a :string @source=|%10$s|} and {$b :string @source=|%2$s|}"
entity = mock_entity("android", string=original)
assert run_custom_checks(entity, original) == {}


def test_android_changed_multi_digit_placeholder():
"""Source XML: Hi %10$s and %2$s

Translation XML: Ciao %11$s e %2$s
"""
original = "Hi {$a :string @source=|%10$s|} and {$b :string @source=|%2$s|}"
translation = "Ciao {$a :string @source=|%11$s|} e {$b :string @source=|%2$s|}"
entity = mock_entity("android", string=original)
Expand All @@ -437,6 +482,9 @@ def test_android_changed_multi_digit_placeholder():


def test_android_protection_matching_element_text():
"""Source XML:
Hi <xliff:g id="a">Name</xliff:g>, see &lt;a title="Name"&gt;link&lt;/a&gt;
"""
original = (
"Hi {$a :xliff:g id=a @translate=no @source=Name}, "
'see {|<a title="Name">| :html}link{|</a>| :html}'
Expand Down Expand Up @@ -593,13 +641,40 @@ def test_xcode_extra_placeholder():


def test_xcode_html():
"""Source XLIFF: Read the &lt;b&gt;policy&lt;/b&gt;"""
original = "Read the <b>policy</b>"
translation = "Leggi la <b>policy</b>"
entity = mock_entity("xcode", string=original)
assert run_custom_checks(entity, translation) == {}


def test_xcode_placeholder_in_element():
"""Source XLIFF: Read the &lt;a href="%1$@"&gt;policy&lt;/a&gt;"""
original = 'Read the <a href="{$arg1 :string @source=|%1$@|}">policy</a>'
translation = 'Leggi la <a href="{$arg1 :string @source=|%1$@|}">policy</a>'
entity = mock_entity("xcode", string=original)
assert run_custom_checks(entity, translation) == {}


def test_xcode_changed_placeholder_in_element():
"""Source XLIFF: Read the &lt;a href="%1$@"&gt;policy&lt;/a&gt;

Translation XLIFF: Leggi la &lt;a href="https://example.com"&gt;policy&lt;/a&gt;
"""
original = 'Read the <a href="{$arg1 :string @source=|%1$@|}">policy</a>'
translation = 'Leggi la <a href="https://example.com">policy</a>'
entity = mock_entity("xcode", string=original)
assert run_custom_checks(entity, translation) == {
"pErrors": ['Element <a href="https://example.com"> not found in reference'],
"pWarnings": ['Element <a href="%1$@"> not found in translation'],
}


def test_xcode_changed_placeholder_in_markup():
"""Source XLIFF: Read the <a href="%1$@">policy</a>

Translation XLIFF: Leggi la <a href="https://example.com">policy</a>
"""
original = "Read the {#a href=|%1$@|}policy{/a}"
translation = "Leggi la {#a href=|https://example.com|}policy{/a}"
entity = mock_entity("xcode", string=original)
Expand All @@ -610,8 +685,15 @@ def test_xcode_changed_placeholder_in_element():


def test_xcode_placeholder_in_and_outside_element():
original = "Visit {#a href=|%@|}{$arg2 :string @source=|%@|}{/a} for details"
translation = "Click {#a href=|%@|}here{/a} for details"
"""Source XLIFF: Visit &lt;a href="%@"&gt;%@&lt;/a&gt; for details

Translation XLIFF: Click &lt;a href="%@"&gt;here&lt;/a&gt; for details
"""
original = (
'Visit <a href="{$arg1 :string @source=|%@|}">'
"{$arg2 :string @source=|%@|}</a> for details"
)
translation = 'Click <a href="{$arg1 :string @source=|%@|}">here</a> for details'
entity = mock_entity("xcode", string=original)
assert run_custom_checks(entity, translation) == {
"pWarnings": ["Placeholder %@ not found in translation"]
Expand All @@ -624,6 +706,10 @@ def test_xcode_placeholder_in_and_outside_element():
)
@pytest.mark.parametrize("count", [0, 1, 2])
def test_repeated_unnumbered_placeholder(format, placeholder, count):
"""Source XLIFF: Hello %@ and %@ (or %#x); Source XML: Hello %s and %s (or %02d)

Translation: Hello / Hello %@ / Hello %@ and %@
"""
first = "{$arg1 :string @source=|" + placeholder + "|}"
second = "{$arg2 :string @source=|" + placeholder + "|}"
entity = mock_entity(format, string=f"Hello {first} and {second}")
Expand Down Expand Up @@ -652,17 +738,25 @@ def test_repeated_unnumbered_placeholder(format, placeholder, count):
],
)
def test_repeated_reusable_placeholder(format, placeholder):
"""Source XLIFF: Hello %1$@ and %1$@ (or %#@items@); Source XML: Hello %1$s and %1$s

Translation: Hello %1$@
"""
argument = "{$arg1 :string @source=|" + placeholder + "|}"
entity = mock_entity(format, string=f"Hello {argument} and {argument}")
assert run_custom_checks(entity, f"Hello {argument}") == {}


def test_repeated_unnumbered_placeholder_outside_element():
element = "{#a href=|%@|}"
"""Source XLIFF: &lt;a href="%@"&gt;%@ and %@&lt;/a&gt;

Translation XLIFF: &lt;a href="%@"&gt;%@&lt;/a&gt;
"""
element = '<a href="{$arg1 :string @source=|%@|}">'
first = "{$arg2 :string @source=|%@|}"
second = "{$arg3 :string @source=|%@|}"
entity = mock_entity("xcode", string=f"{element}{first} and {second}{{/a}}")
assert run_custom_checks(entity, f"{element}{first}{{/a}}") == {
entity = mock_entity("xcode", string=f"{element}{first} and {second}</a>")
assert run_custom_checks(entity, f"{element}{first}</a>") == {
"pWarnings": [
"Placeholder %@ has fewer occurrences in translation (expected 3, found 2)"
]
Expand All @@ -671,6 +765,11 @@ def test_repeated_unnumbered_placeholder_outside_element():

@pytest.mark.parametrize("missing", [False, True])
def test_repeated_unnumbered_placeholder_plural_variants(missing):
"""Source XML: <plurals> with "For %s and %s" for quantity one and other

Translation XML: <plurals> with "For %s" (missing) or "For %s and %s" for
quantity one, few and other
"""
first = "{$arg1 :string @source=|%s|}"
second = "{$arg2 :string @source=|%s|}"
original = (
Expand Down Expand Up @@ -700,6 +799,10 @@ def test_repeated_unnumbered_placeholder_plural_variants(missing):


def test_repeated_unnumbered_placeholder_extra_occurrence():
"""Source XLIFF: Hello %@ and %@

Translation XLIFF: Hello %@, %@ and %@
"""
first = "{$arg1 :string @source=|%@|}"
second = "{$arg2 :string @source=|%@|}"
third = "{$arg3 :string @source=|%@|}"
Expand All @@ -712,7 +815,12 @@ def test_repeated_unnumbered_placeholder_extra_occurrence():


def test_repeated_unnumbered_placeholder_fewer_source_variants():
"""Translations can have fewer plural forms than the source."""
"""Translations can have fewer plural forms than the source.

Source XML: <plurals> with "%s of %s" for quantity one and "%s items" for other

Translation XML: <plurals> with "%s items" for quantity other
"""
first = "{$arg1 :string @source=|%s|}"
second = "{$arg2 :string @source=|%s|}"
original = (
Expand All @@ -726,9 +834,13 @@ def test_repeated_unnumbered_placeholder_fewer_source_variants():


def test_unnumbered_placeholder_moved_outside_element():
"""Source XLIFF: &lt;a href="%@"&gt;%@&lt;/a&gt;

Translation XLIFF: %@ %@
"""
first = "{$arg1 :string @source=|%@|}"
second = "{$arg2 :string @source=|%@|}"
entity = mock_entity("xcode", string=f"{{#a href=|%@|}}{second}{{/a}}")
entity = mock_entity("xcode", string=f'<a href="{first}">{second}</a>')
checks = run_custom_checks(entity, f"{first} {second}")
assert list(checks) == ["pWarnings"]
assert sorted(checks["pWarnings"]) == [
Expand All @@ -739,7 +851,11 @@ def test_unnumbered_placeholder_moved_outside_element():

@pytest.mark.parametrize("source_count, target_count", [(2, 1), (1, 2), (2, 2)])
def test_repeated_element_with_unnumbered_placeholder(source_count, target_count):
element = "{#a href=|%@|}link{/a}"
"""Source XLIFF: &lt;a href="%@"&gt;link&lt;/a&gt;, repeated source_count times

Translation XLIFF: the same element, repeated target_count times
"""
element = '<a href="{$arg1 :string @source=|%@|}">link</a>'
entity = mock_entity("xcode", string=" ".join([element] * source_count))
expected = {}
if source_count != target_count:
Expand All @@ -755,14 +871,24 @@ def test_repeated_element_with_unnumbered_placeholder(source_count, target_count


def test_missing_element_suppresses_unnumbered_placeholder_warning():
entity = mock_entity("xcode", string="Read {#a href=|%@|}link{/a}")
assert run_custom_checks(entity, "Read link{/a}") == {
"""Source XLIFF: Read &lt;a href="%@"&gt;link&lt;/a&gt;

Translation XLIFF: Read link&lt;/a&gt;
"""
entity = mock_entity(
"xcode", string='Read <a href="{$arg1 :string @source=|%@|}">link</a>'
)
assert run_custom_checks(entity, "Read link</a>") == {
"pWarnings": ['Element <a href="%@"> not found in translation']
}


@pytest.mark.parametrize("count", [1, 3])
def test_unnumbered_placeholder_mismatch_in_one_plural_variant(count):
"""Source stringsdict: "%@ %@" for one and other

Translation stringsdict: "%@" or "%@ %@ %@" for one, "%@ %@" for other
"""
argument = "{$arg1 :string @source=|%@|}"
pair = argument + " " + argument
original = ".input {$n :number} .match $n one {{" + pair + "}} * {{" + pair + "}}"
Expand All @@ -780,11 +906,20 @@ def test_unnumbered_placeholder_mismatch_in_one_plural_variant(count):


def test_missing_elements_in_different_source_variants():
"""Source stringsdict: &lt;a href="%@"&gt;link&lt;/a&gt; for one,
&lt;a title="%@"&gt;link&lt;/a&gt; for other

Translation stringsdict: link&lt;/a&gt; for other
"""
argument = "{$arg1 :string @source=|%@|}"
original = (
".input {$n :number} .match $n "
"one {{{#a href=|%@|}link{/a}}} * {{{#a title=|%@|}link{/a}}}"
'.input {$n :number} .match $n one {{<a href="'
+ argument
+ '">link</a>}} * {{<a title="'
+ argument
+ '">link</a>}}'
)
translation = ".input {$n :number} .match $n * {{link{/a}}}"
translation = ".input {$n :number} .match $n * {{link</a>}}"
checks = run_custom_checks(mock_entity("xcode", string=original), translation)
assert list(checks) == ["pWarnings"]
assert sorted(checks["pWarnings"]) == [
Expand Down