This reverts parts of the revert I3bee35f, which reverted a3d312c8.
I believe it's helpful to keep these test cases just to document how
the code currently behaves. I removed all TODO because we don't know
if and how we want to touch this again.
Bug: T240858
Change-Id: Ib91acfcb7292e5c03ce9cc4d7be782085e10aa27
Perform the validation in validateRef, and display a new error message for
broken "follow" refs. This changes existing behavior, where broken folow
ref content is arbitrarily displayed at the top of the references list and
no error is rendered.
Thanks to weasely wording, the new error can later be reused for "extends"
errors.
Bug: T240858
Change-Id: I506e4dcd1151671f5302ecd99581145d979d8124
These create bogus output, depending on the surrounding wikitext the
<ref> tag is used in. For example, this example wikitext:
* Example.<ref name="1">a</ref> More text.
… will be rendered with the "More text" sentence wrapped on the next
line, outside of the list. However, this does *not* happen in many of
the localizations, e.g. German, because many Tanslatewiki translators
did not copied the bogus \n. Why should they.
TL;DR: These newline characters either do nothing, or destroy the output.
In both cases the proper fix is to replace them with spaces.
Some of the test cases touched in this patch demonstrate the issue.
Change-Id: I395a40637a5293eda1f477963d252ce1a215f8b2
This resolves another TODO. Since this is an intentional limitation in
the design of the feature, I find it pretty signigicant to give it it's
own error message.
Note that the text does not need to be perfect, just good enough for now.
We will review all error messages later via T238188.
Bug: T242141
Change-Id: Id9c863061e855350320131e81f6702c8810736f4
"Conflicting" here includes the case where one of two <ref> with the
same name does not have an extends attribute. The first occurence of
a name specifies if a <ref> is a top-level or a sub-reference. This can
not be changed later.
This patch changes multiple existing test cases. I checked all of them
in detail and confirmed the behavior is fine. The error reporting is
better or at least equally good in all cases.
Bug: T242141
Change-Id: Iaec306eefe5b168d496990105e297ca044a5e721
Allow a ref with `name=""` for backwards-compatibility.
Partially reverts I07738cce2641026dfaa92ba263ed6f9834be0944
Bug: T242437
Change-Id: Iaed2d1c41be377a4961aff39838b0965f6c00616
A fun edge case where `name=""` fools both validation branches after
a references rollback, and triggered a LogicException. Stop these
freak refs.
Bug: T242437
Change-Id: I07738cce2641026dfaa92ba263ed6f9834be0944
It's possible to nest <references> by using tricky constructs like the
{{#tag function, and this breaks our rollback logic. Try to show normal
output, otherwise show an error.
Includes regression tests.
Bug: T242437
Change-Id: I33e497cdf8508ce7ccb7f0f315c00af5eee47d0e
Each of these TODOs is something that needs to be fixed or implemented,
so it's helpful to map them to tasks.
Change-Id: I807208392d8a609d7f3b371dc3560a48f3578092
The rollback feature was not able to properly restore a __placeholder__.
That's why a specific use case was behaving different. This already
worked just fine:
<ref extends="a">…</ref>
<references>
<ref name="a">…</ref>
</references>
But this didn't, even if it is the exact same from the users
perspective:
<ref extends="a">…</ref>
{{#tag:references|
<ref name="a">…</ref>
}}
Bug: T239810
Change-Id: I163a1bffb9450a9e7f776e32e66fb08d0452cdb9
Note this leaves *another* bug behind. When a <ref> is properly reused
by name="…", and the content is fine (either missing or identical),
possibly conflicting extends="…" attributes are currently entirely
ignored. However, this is already much better than what happened before.
Bug: T242110
Change-Id: Id808ce31c8036cc290f68bb3e8c5a7b12f4f44cf
This is an extremely relevant use case, but we never had a test for
this:
Some text.<ref extends="book">Page 2</ref>
<references>
<ref name="book">Title of the book</ref>
</references>
What this means: There is no reference in the text that points to the
book as a whole, only references that point to individual pages. The
base <ref> is not used in the text.
This is already properly rendered. There is no "jump back to the text"
link. However, this fails when <references> is wrapped in {{#tag:…}}.
Bug: T239810
Change-Id: Id22db0238266a4fd6131d1a10eb6bf6227552c19
I tried to run these tests with a very old version of this code base
(from 2018) to confirm this is the correct behavior.
Bug: T241303
Change-Id: Id97d016b199458aa178ca732282e9c0e91e291a4
Since I3db5175 the ParserCloned hook handler does not rely on cloning
the Cite object any more. There is no cloning any more. This is dead
code and we could remove it. Just to be sure I propose to keep the
method, but let it throw an exception.
Bug: T240248
Change-Id: I2057ea652ca25f4c7031c28a6e713671738f5e22
Note that this patch changes behavior, an invalid "dir" will result in
a cite reference at the point where the <ref> is declared rather than
in the references section. This is consistent with other errors.
Bug: T15673
Change-Id: Id10db40aa0b391f2f1d9274aa09d22a7278d65e3
This makes one of the last remaining edge-cases about non-empty, but
non-visible content (a <ref> that only contains whitespace) behave
identical to all other places. We already reported it as being empty
everywhere else, except inside of <references>.
Note that the test cases look like they are reporting the same errors
twice. But this is not the case:
The first set of errors is about <ref name="…"> inside of <references>
not having visible content. This should always be reported, even if the
<ref> got content from somewhere else on the page.
The second set of errors is when a <ref name="…"> *never* got any
content.
This patch will slightly increase the numbers of errors reported.
Change-Id: I4a156aa9e466f735d92fe0ba5cc0678ec8bbdd50
This adds a test for numbers like "1.2.0" that appear when an extended
reference (e.g. "1.2.") is reused multiple times.
The first separator is from the extended reference. We decided to never
localize it. However, the second seperator is from reusing a reference.
This was always localized. We believe this is a bug, but haven't fixed
it yet.
The test is documenting the status quo "1.2,0" with a comma. This kind
of makes sense, one could argue, because the "1.2" appears like this up
in the text, but the ",0" is a different indicator for a reuse, which
*never* occurs in the text.
Change-Id: Ie3d26bcadd8929b906bfbcac4806af2150d61f2a
This partly reverts Ied2e3f5. I haven't properly tested this before.
Rendering a bad extends (that extends a <ref> that's already extended)
not indented messes the order up and rips other extended <ref>s out of
context.
For now it might be better to stick to the previous, "magic" behavior:
Such an extends behaves like it is extending the *parent*, and is
ordered and indented as such. This is still not correct, but I feel
this is much better than rendering such a bad extends on the top level.
This patch also makes the code fail much earlier for a nested extends,
if this decision can be made already. In this case the error message is
rendered in the middle of the text (as other errors also are), not in
the <references> section.
Change-Id: I33c6a763cd6c11df09d10dfab73f955ed15e9d36
These edge cases are handled correctly already, I just forgot to
remove the TODOs when updating test content.
Note that there's only one TODO left, and it's to forbid a feature which
actually works!
Change-Id: I0d3a1f55f0ce943b0d034dda40e3779fbf241fe4
The new extends="…" feature is using numbers like "1.2". These should be
localized in languages like Hebrew that uses other symbols for the digits.
But the "." should not change.
The existing feature when a <ref> is reused multiple times does have the
same "issue". But it seems this is intentional, because it is covered by
a test. Note this is not visible in German, because German uses custom
labels "a", "b", and so on.
This patch also improves the so called "smoke" tests and makes one cover
numbers up to "1,10" for a <ref> that is reused that often.
Bug: T239725
Change-Id: Iffcb56e1c7be09cefed9dabb1d6391eb6ad995ce
If `extends` is encountered before the parent ref, we reserve the
sequence number and leave a placeholder to record the link between
ref name and number. This is necessary to render a list like,
"[1] [2.1] [2]", or to use subreferencing when the parent ref is
declared in the references tag.
When a placeholder is encountered during references section rendering,
it means that the parent was never declared.
Change-Id: I611cd1d73f775908926a803fae90d039ce122ab6
This was carrying the entire footnote marker, but subreferences need
to extract just the first (group ref sequence) part. Storing number
and extendsIndex in two separate fields gives us more flexibility
during rendering, for example these might use two different symbol sets.
Change-Id: I75bd6644c336036f9e84ba91e1c35e05bc1ca7f3
