Commit graph

90 commits

Author SHA1 Message Date
Adam Wight f2bd6b6dcc Revert "Standardize "follow" validation"
This reverts commit a3d312c8f4.

Bug: T240858
Change-Id: I3bee35f27797a04c41c265f7e598d8383414b67a
2020-02-05 11:42:28 +01:00
Adam Wight a3d312c8f4 Standardize "follow" validation
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
2020-01-30 17:25:42 +00:00
jenkins-bot 3c8a225052 Merge "Fix incomplete rollback producing bad footnote numbers" 2020-01-24 15:11:31 +00:00
Thiemo Kreuz 2ddc6f133b Fix incomplete rollback producing bad footnote numbers
Bug: T48140
Change-Id: I53ce5d8488d4c24d6f23f6f0e70806d7db4064e1
2020-01-24 13:02:53 +01:00
Thiemo Kreuz 816b1b0add Remove newline characters from all error messages
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
2020-01-24 12:29:14 +01:00
Thiemo Kreuz 51d55bb8de Introduce dedicated error message for nested <ref extends=…>
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
2020-01-23 15:00:26 +01:00
Thiemo Kreuz 9565d6e887 Resolve a TODO by covering it with a test case
It turns out this is indeed necessary. The test demonstrates why.

Change-Id: Id9c6a48f72ef8d3f0cc9a714d826418e69913b0a
2020-01-21 10:11:03 +00:00
jenkins-bot 258b23a6dd Merge "Error when reusing <ref> with conflicting "extends" attributes" 2020-01-20 13:49:41 +00:00
Adam Wight 8a58ed55dc Error when reusing <ref> with conflicting "extends" attributes
"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
2020-01-20 13:33:52 +00:00
Adam Wight b3ea9f4ef8 Relax empty-string name validation
Allow a ref with `name=""` for backwards-compatibility.
Partially reverts I07738cce2641026dfaa92ba263ed6f9834be0944

Bug: T242437
Change-Id: Iaed2d1c41be377a4961aff39838b0965f6c00616
2020-01-20 12:40:09 +00:00
Arlo Breault 7e2bbae4c2 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 0a6c576ad6ccfc81c2bfa20757417c62e554ef56

Change-Id: Ifa5d60e362e5c530d12d3b94351aef2d1b1962cc
2020-01-17 14:51:03 -05:00
Adam Wight f3031b80b9 Fix for blank-named ref in #tag
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
2020-01-17 11:19:29 +01:00
Adam Wight 1c947a808d Fix for nested #tag:references
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
2020-01-15 12:44:29 +01:00
jenkins-bot 09f4deede4 Merge "Replace now unused native cloning feature" 2020-01-09 14:13:58 +00:00
Adam Wight 170484e933 Annotate TODOs with task number
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
2020-01-09 13:13:48 +01:00
jenkins-bot 0be582dc12 Merge "Report conflicting extends="…" with an error message" 2020-01-09 11:37:27 +00:00
jenkins-bot b9b6905171 Merge "Fix incomplete undo/redo stack implementation" 2020-01-09 10:58:35 +00:00
Thiemo Kreuz 04fbbbd3ca Report conflicting extends="…" with an error message
Bug: T242110
Change-Id: I04342b2c219981dfb9575ea58cfccf6c2ba1066c
2020-01-08 16:47:07 +00:00
Thiemo Kreuz d07110b790 Fix incomplete undo/redo stack implementation
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
2020-01-08 17:43:02 +01:00
Adam Wight b7c9dbb0d5 Remove invalid test case
Unnamed references are never merged.

Bug: T239788
Bug: T240459
Change-Id: I8dd3706c688108bf2e3c0e9b55f123084b325d16
2020-01-08 16:59:28 +01:00
jenkins-bot 861c4edba7 Merge "Test cases for extends pointing to the <references> section" 2020-01-08 10:42:14 +00:00
Thiemo Kreuz 6ddfd9983b Fix bad numbering when reusing sub-references
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
2020-01-07 16:34:05 +01:00
Thiemo Kreuz 5db90fb5a9 Test cases for extends pointing to the <references> section
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
2020-01-07 12:43:18 +01:00
Thiemo Kreuz 38d5bd5f39 Add missing parser tests for relevant responsive edge cases
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
2019-12-28 20:59:23 +00:00
Thiemo Kreuz 0dc6f37785 Replace now unused native cloning feature
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
2019-12-20 20:07:59 +01:00
jenkins-bot 45119f8c61 Merge "Move "dir" error handling to validation" 2019-12-19 10:18:24 +00:00
Arlo Breault 6d55f9e8cc Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 41f397ce4d563fa7f7770725d88944dcabda4116

Change-Id: I27b7f035c8b99ca80501b8cd1169ed8c8895ef93
2019-12-18 15:30:49 -05:00
jenkins-bot 0d7e04e1ee Merge "Fix inconsistent error reporting for invisible content" 2019-12-18 09:27:03 +00:00
Adam Wight 1e82f8f073 Move "dir" error handling to validation
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
2019-12-18 10:05:59 +01:00
Thiemo Kreuz 1f76199ed8 Add parser tests for the responsive="…" feature
Change-Id: Id9d733dabf82f2c26f51c6fbd1e03fe0574e88a8
2019-12-17 15:51:41 +01:00
Thiemo Kreuz 1bd66081f7 Fix inconsistent error reporting for invisible content
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
2019-12-17 13:37:01 +01:00
Thiemo Kreuz f86b5073fd Add parser tests for reused extended <ref> before defined
Bug: T240424
Change-Id: I945c2e12cfa3ff851380a1ff4491c8af076f523a
2019-12-11 16:30:17 +01:00
Thiemo Kreuz 193b840010 Add parser test for duplicate extended references
Bug: T240459
Change-Id: Ifc7a695e89a49ccc6c66d49efe41b2321b0915f0
2019-12-11 15:58:34 +01:00
Adam Wight 084ca6b3d4 Integration parser test for ParserFunctions
Depends-On: I09844079f163e583d3b1e941c701f8cda5029a0a
Bug: T240345
Change-Id: I86c55ff88d9f4b800e8868728dfec6b4ceda82c2
2019-12-11 08:58:58 +01:00
Thiemo Kreuz f92792f64a Fix bad localization of extended references numbers when reused
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
2019-12-09 17:25:14 +01:00
Adam Wight f51060eaf4 Fix footnote mark after extends numbering glitch
The visible numbering needed to be rolled back after an extends.

Bug: T237241
Change-Id: I95404515110df1fa7e3279ea499577df0ed45ddf
2019-12-09 12:06:59 +01:00
jenkins-bot 3b41cfa472 Merge "Fail early on nested extends="…", if possible" 2019-12-09 10:12:54 +00:00
jenkins-bot 399a9c63bf Merge "Numbering bug: Parser test which should fail" 2019-12-09 10:06:06 +00:00
Thiemo Kreuz c5fe49ff11 Fail early on nested extends="…", if possible
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
2019-12-09 10:54:52 +01:00
Adam Wight a91cf43154 Remove outdated TODOs
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
2019-12-09 10:25:19 +01:00
Adam Wight d8433101a7 Numbering bug: Parser test which should fail
Includes the TODO for what correct output looks like.

Bug: T237241
Change-Id: I0e60724f2c418b19e5affc24dca7f446c2b38bb3
2019-12-09 09:53:53 +01:00
Thiemo Kreuz 31bda4777b Don't indent refs with forbidden extends="…"
Change-Id: Ied2e3f56ce66d2a8ccf60df2bdbf99acad461595
2019-12-04 15:17:03 +00:00
Adam Wight 81261493c2 Show error when extending a subreference
Change-Id: Iaa47e302e5e49dfc190fde37567a3e7a2e743d67
2019-12-04 13:49:31 +01:00
Thiemo Kreuz 54333c9bd6 Stop formatting "1.2" as "1,2" in languages like German
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
2019-12-04 09:43:04 +01:00
Adam Wight 008526b3aa Can use extends before its parent
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
2019-12-02 17:14:11 +01:00
jenkins-bot fa4410836d Merge "Split ref.number field" 2019-12-02 15:29:53 +00:00
jenkins-bot 79e7b2b474 Merge "Add test cases for duplicate <references> with same group" 2019-12-02 15:26:02 +00:00
Thiemo Kreuz 2cb7e5d438 Add test cases for duplicate <references> with same group
Change-Id: I9603e7ebf167330b1eddae1676e9234edf6557bc
2019-12-02 15:08:15 +00:00
Adam Wight 3f276388bf Split ref.number field
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
2019-12-02 10:17:24 +01:00
Adam Wight 00f3be7c7f Reset extendsCount after each group
This was a bug which would affect book references, if the same group
and parent ref name combination occur twice in an article.

Change-Id: I608f58aac0cec31c8650835fc80195a87bc851d3
2019-12-02 10:17:18 +01:00