Commit graph

466 commits

Author SHA1 Message Date
thiemowmde c207e343a2 Add test case with conflicting dir="…" values
Just to document the current behavior.

Bug: T202593
Change-Id: I87c4118cd8ca9f860319dc1d3a25f448019339c4
2023-03-01 08:54:56 -06:00
thiemowmde 899775b5ce Remove problematic spaces from a parser test case + Rename a test file
IDEs like my PHPStorm trim spaces from the end of the line. It looks
like they are not relevant for the test and can as well be removed.

Change-Id: I54cb4fdf74dd7174450dcc552b077d388dbac749
2023-03-01 08:54:11 -06:00
thiemowmde b2be6f4ece Update incomplete ReferencesFormatterTest
The mock was incomplete, leaving a feature uncovered.

Change-Id: I657728f89c94602a7aa9b3fbfabc53e7fdac348c
2023-02-27 17:40:56 +01:00
thiemowmde 6f302bf3f0 Replace string|false in CiteParserTagHooks with nullable ?string
This allows us to use strict types in one more place.

Change-Id: Id31a427b61a5eca720ec695fe219809a8e37d208
2023-02-27 15:32:04 +01:00
Arlo Breault 1dd994a28a Re-enable test
Change-Id: I9c909e10170a0437676bdf1290cc6421073a8284
Follow-Up: If9cdabdfac26656272fcf3b4aaae0576aaed1346
Depends-On: If1e55feb86ce8b32f772e3b78bc9d29f122f4d58
2023-02-09 17:30:52 -05:00
Arlo Breault 5fa75277b7 Disable test for CI
Needed-By: If1e55feb86ce8b32f772e3b78bc9d29f122f4d58
Change-Id: If9cdabdfac26656272fcf3b4aaae0576aaed1346
2023-02-09 17:28:08 -05:00
Arlo Breault d5b5a83ae9 Sync up Cite repo with Parsoid
This now aligns with Parsoid commit b520b8092db6f092603e7244cdac5f1fc645e89a

Change-Id: I671c5831d90bbeb088f5ff2133df553142e1af53
2023-02-09 16:58:34 -05:00
Arlo Breault 0060e2b43d Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 9041d0bac69fe763112c0d8fcbd553a211a25c26

Change-Id: I34b1fdf2232e67db5326ed25ab3f4ad128e59e5d
2022-08-23 15:11:06 -04:00
Thiemo Kreuz 8fef0dd2aa Improve two error messages
Makes it easier to find the source of the error in the wikitext.

Change-Id: I648a19881210184ab1abe9b948b5efbbbdabcdc9
2022-08-20 12:23:28 +02:00
Kosta Harlan e82b3c8a76
phpunit: Unit tests may not access MW services
Bug: T266441
Change-Id: Iab4f2e76daddeb88d018f4ead86f26fc62448e24
2022-07-13 10:10:10 +02:00
Thiemo Kreuz 11255770c5 Make message key parser accept more than just underscores
The best practice for message keys is to use dashes, not underscores.
This codebase is quite old and traditionally uses underscores. I
think we can make it flexible enough to work with both.

Required for Ie64f4ab.

Change-Id: I6f0584299a4f279ed929784927392eb0f72cbc80
2022-06-01 10:34:57 +02:00
C. Scott Ananian d730d17c1a parser tests: move test which requires {{#ifeq}} into its own file
Since parser test requirements are per-file, move the smoke test which
requires `{{#ifeq}}` (from [[mw:Extension:ParserFunctions]]) into its
own file and define the requirement properly in the file header.

That avoids spurious parser test failures if developers don't have
the ParserFunctions extension installed locally.

Change-Id: Ia5ffbe0896d5033fe2da526e42bf111edbc56adf
2022-05-27 11:38:02 -04:00
jenkins-bot 20ec7ced59 Merge "Use new ResourceLoader namespace" 2022-05-24 23:22:32 +00:00
Tim Starling bb72bc65f5 Use new ResourceLoader namespace
Extensions using Phan need to be updated simultaneously with core due
to T308443.

Bug: T308718
Depends-On: Id08a220e1d6085e2b33f3f6c9d0e3935a4204659
Change-Id: Iebc5768a3125ce2b173e9b55fc3ea20616553824
2022-05-24 23:00:08 +00:00
Subramanya Sastry 5515de2003 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit e4305217923162568cf5b6ec08ba3c96fd5b04e3

Change-Id: I935d8641d90ec5cbcf7d29fd984085cc5338b6ec
2022-05-17 18:00:18 -05:00
C. Scott Ananian af2352e523 parser tests: Make !! config values JSON-compatible
Bug: T307720
Change-Id: Ib716c70bc47659701edfc572674b3e890e19605b
2022-05-11 21:05:55 -04:00
Subramanya Sastry 45cc963ca2 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 12d896bd5852b4a7d602fb22dd09d7fc2c5c5b64

Change-Id: I1d3183ee4afa37e0d71768cd02f03112771b82c4
2022-05-11 18:08:24 -05:00
Aryeh Gregor 7bd68f0efa Avoid indirect global access in unit tests
If a ResourceLoaderFileModule is constructed with no arguments, it
accesses global variables, so this is not allowed from a unit test.
(This is probably a bug in ResourceLoaderFileModule, but one thing at a
time.)

This blocks If005958c76bbfabba74def4215c48fe94f297797.

Change-Id: I84056024b0d3a9dcddb1ab4dc8596118bb3fe8ea
2022-04-27 18:10:29 +03:00
Subramanya Sastry a478d6c3a3 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit c434df9.

Change-Id: Ib8b8593d1a303530dac56ed01335e0d635863eda
2022-04-20 19:03:18 -04:00
Subramanya Sastry 169ebf6344 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 3b5a7a318eaf8dcc5e6ed142352f1cf1a9439474

Change-Id: I32f9fcac231cdf25bd1501d7f6e022e067c8d8e1
2022-04-14 12:15:53 +05:30
C. Scott Ananian e7dd9b5549 Page properties should always be strings
Bug: T305158
Bug: T237531
Change-Id: Ibfd84b52057baa8e249d321ec9df612efd6a29a6
2022-03-31 14:26:40 -04:00
Thiemo Kreuz 7b30a165e4 Use correct Sanitizer method for id/fragment escaping
Note how only the HTML5 mode behavior changes, but nothing in
legacy mode.

Also note this does not 100% fix the issue. The esample with a
non-breaking space is still broken. But it's already much better
than before.

Bug: T298278
Change-Id: Idf50dad4219ff4c594a0cc15f63cb10fdac5ffb7
2022-01-03 16:23:45 +01:00
Thiemo Kreuz 83041449a7 Add parser & unit test cases for different $wgFragmentMode's
This is only to document the current status quo and make later patches
smaller and easier to review.

Bug: T298278
Change-Id: I6c78f4d3ee32de596f2b5ee081d56eaffb1cc7bd
2022-01-03 14:14:47 +01:00
Timo Tijhof bee357337c ve-cite: Export citationTools as native object instead of JSON string message
* Remove the overhead of serializing and then re-parsing client-side,
  instead assign it directly as native object literal.

* Move code for array slicing to PHP.

Change-Id: Iedcc8d57d3bddd3fa32a78b4e7ecc25615d94277
2021-12-07 12:53:24 +00:00
Timo Tijhof fb2c7c37db Combine ext.cite.visualEditor.data into ext.cite.visualEditor
Rather than depending on a separate module with one line of generated
JS code, generate it as a prepended statement to the same module.

Should be a no-op.

Depends-On: I809951d34feb2dbd01b7ae0f4bd98dac7c3f6fe2
Change-Id: I5886bf9f82025048976b7750e8cb751681021fb4
2021-11-19 16:56:51 +00:00
Ed Sanders cb60e7aa04 build: Update eslint-config-wikimedia to 0.21.0
Change-Id: I86a44d7c73a107fb318abeda9e503e99083f48db
2021-11-09 14:25:34 +00:00
C. Scott Ananian 30cfb7c05a Rename deprecated usage of ParserOutput::{get,set}Property()
Bug: T287216
Depends-On: Ie963eea5aa0f0e984ced7c4dfa0fd65d57313cfa
Change-Id: Id4581c6c45f9fc4690900a30d8172951bc461a1b
2021-10-08 10:12:05 -04:00
Subramanya Sastry ed59e2ac38 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 29f8e7051529ecbb62fc52bff6726a4df8bf20c2

Change-Id: I165ee24e1b78bdf181fa45430fdec1549310c359
2021-09-30 15:00:56 -05:00
vladshapik 71c4bd1f98 Adjust Parser related tests to DeprecationHelper
There is the patch(I4297aea3489bb66c98c664da2332584c27793bfa) which will
add DeprecationHelper trait to Parser class in order to deprecate public
Parser::mUser. DeprecationHelper trait has appropriate magic methods
which help to use dynamic properties. In order not to mock them via
createMock(), so getMockBuilder() and onlyMethods() was used.
onlyMethods() method helps to specify methods which need to be mocked.
Now we can use dynamic properties in Parser related tests of Cite
extension.

Bug: T285713
Change-Id: Ie75c9cd66d296ce7cf15432e2093817e18004443
2021-08-17 14:14:55 +00:00
C. Scott Ananian 53e8dc7b39 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 88d4620278988d121761fb440952d1d66a70ce99

Required some newline fixes to resync after "Refactor newline logic"
(change I6691c70f8e3fa3f21e2d11035bed9cdc2dc87093 /
commit 6389459b1e) was merged
this morning.

Change-Id: I64fba6cc9330a55d4e1eeb5371164b3eb4efa508
2021-07-30 11:14:38 -04:00
jenkins-bot e584c5cf3f Merge "Refactor newline logic for auto-generated <references> sections" 2021-07-30 14:17:49 +00:00
libraryupgrader 0af1a039b0 build: Updating mediawiki/mediawiki-codesniffer to 37.0.0
Change-Id: Ia399fafc5280aab1c0cfc129529a822e8d8f8382
2021-07-22 12:34:32 +00:00
sahil d7ad615e67 selenium: Update wdio-mediawiki
wdio-mediawiki v1.1.1:
- Includes wdio-defaults.conf.js file that vastly simplifies wdio.conf.js.
- Replaces @wdio/spec-reporter with @wdio/dot-reporter.
- Introduces video recording.

Bug: T283597
Change-Id: Ic62db3ca745a94573b2b0500f49a45bb2a0dcd4f
2021-06-09 15:03:58 +02:00
sahil d2ef9166ae selenium: Update README.md file
Bug: T282237
Change-Id: I45502311712f832f19ab236a9e71081492de224c
2021-05-24 18:24:32 +00:00
libraryupgrader 3d79d3518b build: Updating composer dependencies
* mediawiki/mediawiki-codesniffer: 35.0.0 → 36.0.0
* php-parallel-lint/php-parallel-lint: 1.2.0 → 1.3.0

Change-Id: I409d4c564a02f537dbd1ceec1576410aefe0ea64
2021-05-04 01:29:20 +00:00
libraryupgrader be7f1b3bd7 build: Updating dependencies
composer:
* mediawiki/mediawiki-codesniffer: 34.0.0 → 35.0.0
* mediawiki/minus-x: 1.1.0 → 1.1.1

npm:
* eslint-config-wikimedia: 0.17.0 → 0.18.1

Additional changes:
* Added the "composer phan" command to conveniently run phan.

Change-Id: I2e27a8ae5547829501c25402da5b72b390897ca1
2021-01-29 06:21:06 +00:00
Umherirrender e30f055226 Improve function and property documentation
Removed some wrong @param from test function,
the @dataProvider should be a enough here

Change-Id: Ib84ce497fef4d48df7547ebc38515fc377e7de01
2021-01-16 13:44:19 +01:00
Taavi Väänänen 9293e2d5ca Update Cite to use the new HookContainer/HookRunner system
Bug: T270875
Change-Id: Ieb29603cde24a1c52829c12ae431eca09ba37bf9
2020-12-28 17:55:34 +02:00
Ed Sanders 35b704dc2f Move ext.cite.ux-enhancements to extension.json
Change-Id: Ia6a0c34800b018e76b9e246898ddfb991f238d55
2020-12-14 10:38:32 +00:00
Thiemo Kreuz d44754168f Fix coverage report for ErrorReporter
The structure of this class changed a lot in If2fe5f5
(T239572). This was never reflected in the @covers tags. I
suggest to go with a trivial @covers for the entire class
and let PHPUnit figure it out. The alternative is to kind
of repeat many private implementation details, and this
doesn't feel right.

Change-Id: Ie414876489133ab9aca934c19a5e403cd339abf1
2020-10-27 09:49:56 +00:00
Thiemo Kreuz 27c5632ebd Compact trivial arrays in test providers
This is meant to make the code easier to read.

Change-Id: Ib684bf64acfbac9b458d110d9ccff2d071b993be
2020-10-08 11:07:35 +02:00
Thiemo Kreuz 7ce27b432f Stop using Language::formatNum to localize separators
This is done to make the discussion in If3dcfd7 easier.

When we introduced this code we actually used it to format
entire numbers. We had to change this later to *not* localize
digits, but only separators. Language::formatNum is and always
was able to do this, so we just continued to use it.

This is discussed now.

It turns out there is only a single place left where we use
formatNum, and it does nothing but localizing the decimal
point. There is another way to do the same.

Bug: T237467
Change-Id: I89b17a9e11b3afc6c653ba7ccc6ff84c37863b66
2020-09-02 09:40:33 +02:00
C. Scott Ananian 6b813c6874 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit d9a3e14dfcb422e95de7a79f0eb662fd43f9354f

Change-Id: Iae34d2107bfb47304819da7f7c715dec83da1a48
2020-08-18 16:54:33 -04:00
C. Scott Ananian a0faedb942 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 77db605163990ad851e3da0fb4fa7eca2081f379

Change-Id: I19ad9f1b6845e5557c2f5f87ac435db0ad87000d
2020-07-28 15:53:08 -04:00
Thiemo Kreuz 41149d8072 Restore preview of a <references> section alone
We broke this feature in December 2019 because it was never covered
by any tests. Full explanation in T245376.

All the features we care about are covered by tests. If all existing
tests succeed, that should be proof enough that this patch does not
introduce any new regression.

Bug: T245376
Change-Id: I1a447884bdc507ac762d212466496b4591c18090
2020-07-21 13:19:38 +02:00
C. Scott Ananian 90728bde87 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 7321ab547b7663ba86c1cfe0bc021ff1918c0970

Change-Id: I2cc88069b19e7611f23c83ca993f9caa70f786f0
2020-07-15 11:39:46 -04:00
vidhi-mody 76c38c766d Selenium: Update to WebdriverIO v5
Update NPM packages: webdriverio, wdio-mediawiki.

Replace NPM packages:
- wdio-mocha-framework with @wdio/mocha-framework.
- wdio-spec-reporter with @wdio/spec-reporter.

New NPM packages: @wdio/cli, @wdio/local-runner, @wdio/sync.

Replace:
- `browser.element` with `$`.
- `browser.elements` with `$$`.
- `chromeOptions` with `'goog:chromeOptions'`.
- `password` with `mwPwd`.
- `username` with `mwUser`.
- `waitForVisible()` with `waitForDisplayed()`.
- `isVisible()` with `isDisplayed()`.

Bug: T253343
Change-Id: Ia656c8bc9fa76ae80bc356dc18c821a93b8cd875
2020-06-25 01:19:38 +05:30
jenkins-bot 40e123fb35 Merge "build: Update devDependencies" 2020-06-09 11:16:00 +00:00
Ed Sanders 85a4e23008 build: Update devDependencies
Change-Id: I38b506d6e058f639e0e7d95c3e60616dbef5af10
2020-06-09 11:29:03 +01:00
Bartosz Dziewoński 1975cb3dcb Do not add thousands separators when formatting reference numbers
Bug: T253743
Change-Id: I8c4de963277895d7751d6bfe3c34ca6097ebe606
2020-05-28 00:08:44 +02:00
Thiemo Kreuz 7fbd5de7f5 Merge two code paths related to follow
This patch also adds a test case that was missing before. If a
follow="…" is followed by another, normal <ref>, the internal key
(a.k.a. $this->refSequence) is not incremented. This was the case
before, just not covered by any test.

Change-Id: I102d1e67a6918017acc7e4a4663b08c828d101a6
2020-05-12 10:52:08 +02:00
jenkins-bot 53cf713cdb Merge "Add test cases for impossible follow vs. rollback edge case" 2020-05-12 02:03:35 +00:00
James D. Forrester c35d47fe0b Use QUnitTestModule instead of deprecated ResourceLoaderTestModules
CI already ensures that VisualEditor is loaded alongside Cite, so
the defensive check in the code isn't needed; ext.cite.visualEditor is
defined statically, it's just injected into the page dynamically in the
VisualEditor code handling VisualEditorPluginModules.

Bug: T232875
Change-Id: Ie5e096feca92f9c3ef13c732f3f1ae491e2b7d03
2020-05-11 20:51:24 +00:00
Subramanya Sastry eb8e07c69c Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 1e72ddefc2df8d5b38d7b370d67a87b48d8a09f7

Change-Id: Ic75ac33c92997427751d037339f7eb79027356a2
2020-05-07 12:14:19 -05:00
jenkins-bot fb4fddfb89 Merge "Add a newline in wikitext before autogenerated reflist" 2020-05-06 20:10:45 +00:00
Thiemo Kreuz 6389459b1e Refactor newline logic for auto-generated <references> sections
This change does have two effects:

1. Instead of prepending a newline individually in every possible
code path, we do it one time at the end. But only if there is
something in the output. This does not change anything, as proven by
the unchanged parser tests.

2. I removed the newline between the <h2> and the generated
<references> element. Note that both these elements are created in
the same method, next to each other. So there is no way this can
influence other wikitext. Unfortunately this code path is executed
only when using the *preview* function, and impossible to be covered
by parser tests because of this. However, it's covered by unit tests.

This refactoring is motivated by, but not required for T148701.

Bug: T148701
Change-Id: I6691c70f8e3fa3f21e2d11035bed9cdc2dc87093
2020-05-06 19:07:40 +00:00
Bartosz Dziewoński 3678215a77 Add a newline in wikitext before autogenerated reflist
Previously the reflist was added at the end of the last line of text,
which messes up paragraph wrapping (as seen in many test cases), and
generated invalid HTML when the last line was a list item (T148701).

(second try, previously reverted in 8c933d03c5)

Note this affects only pages where the <references /> tag is missing,
and the references section is auto-generated at the very end of the page.

Bug: T148701
Change-Id: Ib2101346434a4e317b5fc7379215b60c7020cb2b
2020-05-06 20:51:25 +02:00
jenkins-bot 99d5ad6d36 Merge "eslint: Remove unused rules" 2020-04-25 18:50:07 +00:00
Ed Sanders bd8010c360 eslint: Remove unused rules
Change-Id: Iaba2e171da4a969454de1006883a5320a402d9c8
2020-04-24 22:19:27 +01:00
C. Scott Ananian 376c0418d3 Update parser tests to v2 (tidy by default)
The most common cleanup required by switching to tidy output was adding
missing <p>-wrappers to the last item before <references/>.

Bug: T246285
Change-Id: I7c8a08c4e6eff7caf4539a26fae475a4133f9a0c
2020-04-01 11:11:13 -04:00
Thiemo Kreuz a00b8d990d Add test cases for impossible follow vs. rollback edge case
While working on the patch I4303642 I was worried about the line

array_pop( $this->refCallStack )

in the rollback code. Since the patch changed the position of follow
elements in the stack, an array_pop() would pop different elements.

It turns out this is impossible. Rollbacks are only done for <ref>
elements inside a <references> tag, immediatelly after reaching the
closing </references>. It's impossible to use follow="…" inside
<references>. It will not be added to the stack, and therefore not
rolled back.

Even if the edge case would be possible, the *old* code that placed
follow elements on the *other* side of the stack would have been
wrong then.

The test cases in this patch try to hit this edge case, and are
expected to not be able to do so.

Change-Id: I4380bf443db17c6214dbfa2cbda62b46db04258a
2020-04-01 09:03:19 +00:00
jenkins-bot a27b4c82a6 Merge "Revert "Add a newline in wikitext before autogenerated reflist"" 2020-04-01 08:15:41 +00:00
Awight 8c933d03c5 Revert "Add a newline in wikitext before autogenerated reflist"
This reverts commit 90697ffe43.

Change-Id: I659ce1689603fd16e378fb8d3d5bd6d1089342b2
2020-04-01 08:03:55 +00:00
jenkins-bot 45f4990e34 Merge "Add a newline in wikitext before autogenerated reflist" 2020-03-31 19:37:34 +00:00
Bartosz Dziewoński 90697ffe43 Add a newline in wikitext before autogenerated reflist
Previously the reflist was added at the end of the last line of text,
which messes up paragraph wrapping (as seen in many test cases), and
generated invalid HTML when the last line was a list item (T148701).

Bug: T148701
Change-Id: Ifc873fc913e717026d80d54b570c594d1073fb42
2020-03-31 19:00:51 +00:00
jenkins-bot a00401fdfc Merge "Remove not needed code without changing anything" 2020-03-31 18:30:21 +00:00
Thiemo Kreuz 53b043f28f Remove not needed code without changing anything
This removes a few tiny pieces of code, and a large chunk related to
incomplete follow="…" attributes (see T240858). It turns out we don't
need to insert elements at the top of the ReferenceStack::$refs
array, because this array is reordered anyway in
ReferencesFormatter::formatRefsList()!

Incomplete follow refs don't have a number, and are ordered to the top
because of this, as before. This doesn't change with this patch.

Change-Id: I43036420be22feb8f0f287d9ccee2afd317df2a9
2020-03-31 18:15:14 +02:00
Timo Tijhof ce27a400e1 CiteHooks: Remove wgResourceModules check (redundant with isModuleRegistered)
The isModuleRegistered() method was introduced a few years ago,
when the load order in ResourceLoader was undergoing a change.

It used to be that hooks like were run first to register modules, and then
wgResourceModules was registered afterwards. This was reversed to disallow
mutating the config at run-time from foreign modules and to allow better
caching and error detection.

It's been several years since then, so this redundant check is no longer
needed. ServiceWiring.php in MW core for ResourceLoader always processes
config and extension.json first before this hook is called.

Bug: T247265
Change-Id: I466f1fa70b8f0e9fe5e8e8df90bb0001b3329b87
2020-03-10 16:18:46 +00:00
Brian Wolff 5ed973a49a Fix unit tests for whitespace change in Html5 fragments
Bug: T238385
Depends-On: Ie2b7c9429691e2c491c3359d5b400d8f078aa789
Change-Id: Ie2b7c9429691e2c491c3359d5b400d8f078ab111
2020-02-25 13:56:53 +00:00
Brian Wolff 1c6a92ca9b Temp disable test to work around circular dependency in unit test
Html5 fragment mode now bans whitespace per html5 spec

See Ie2b7c9429691e2c491c3359d5b400d8f078aa789

Change-Id: Ie6fa40798f06a358f6082110b4d8cc0028c80321
2020-02-25 04:38:06 -08:00
jenkins-bot 7ee2e81ade Merge "Add two extreme follow edge cases back to parser tests" 2020-02-11 09:11:20 +00:00
Thiemo Kreuz 400ce89f30 Don't talk about follow being "broken" but "incomplete"
Bug: T240858
Change-Id: Iab6563fdf19d6e85795911e4140476fceabf7334
2020-02-05 16:38:49 +00:00
Thiemo Kreuz 48e2f02e20 Add two extreme follow edge cases back to parser tests
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
2020-02-05 15:04:49 +01:00
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 b15f1b81a0 Revert "Remove "follow" special case from ReferencesFormatter"
This reverts commit 38122d91cd.

Bug: T240858
Change-Id: I7198d5534acded94bc83962262c4cdfed9782454
2020-02-05 11:42:27 +01:00
Adam Wight eb799af3df Revert "Remove broken "follow" special case from ReferenceStack"
This reverts commit d01cba60fb.

Bug: T240858
Change-Id: I5b528a285ed6a658ceb333b58f0f4a81a64c7f15
2020-02-05 11:42:26 +01:00
jenkins-bot 7c3ace4fef Merge "Rewrite ErrorReporter for performance and separation of concerns" 2020-02-03 14:58:55 +00:00
jenkins-bot 74fab9755f Merge "Remove one unnecessary LogicException from ReferenceStack" 2020-02-03 14:50:40 +00:00
Thiemo Kreuz d80bd3ef97 Rewrite ErrorReporter for performance and separation of concerns
This patch is mostly moving code around without changing the behavior.
Exceptions:

* The ErrorReporter creates a <span> container. This was previously
parsed. The only benefit might be error checking and escaping. Rather
pointless. The code just created this HTML. With this patch, it is not
parsed any more. The unit test reflects this change. The output in
production will not change, as the parser tests show.

* Parsing of the message key (to detect it's type and id) is simplified
a lot, using explode. With this the code can, in theory, support more
types.

Bug: T239572
Change-Id: If2fe5f55db46dfc7e0ce445348608bef00bec64e
2020-02-03 15:23:40 +01:00
Adam Wight d01cba60fb Remove broken "follow" special case from ReferenceStack
This is unreachable now that broken follow refs fail validation.

Bug: T240858
Change-Id: I22adaee9c4eaeb94bee953ae15c642e044b6a54b
2020-02-03 12:27:59 +01:00
Adam Wight 38122d91cd Remove "follow" special case from ReferencesFormatter
This is unreachable, now that broken follow refs fail validation.

Bug: T240858
Change-Id: I77faeaac4bc53632ab8b82bff7e335ee8c99dfa5
2020-02-03 12:27:57 +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
Thiemo Kreuz 0fda08b25a Remove one unnecessary LogicException from ReferenceStack
This exception was introduced very late in the patch I38c9929. It
already caused trouble. This here is essentially a revert. It restores
the previous behavior where this edge-case was silently ignored. The
worst thing that can happen is that appendText() creates an incomplete
entry in the $this->refs array, which will be rendered at the end. The
user can see it then.

As of now we are not aware of a code path where this would even be
possible. Still this does make the code *more* robust by not making it
explode, but give the user something they can work with.

Bug: T243221
Change-Id: I2e2d29bbd557090981903fcc2ece8796fafa4aa4
2020-01-28 16:15:55 +01: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
jenkins-bot 84341c3603 Merge "Replace ReferenceStack mocks with actual instances" 2020-01-21 11:16:30 +00: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 8700177736 Merge "Use StatusValue::isGood() instead of isOK()" 2020-01-20 16:26:29 +00:00
Thiemo Kreuz 6a4a0fd013 Replace ReferenceStack mocks with actual instances
… if possible. In most cases it's possible to use the real object, and
reach into it's private parts via TestingAccessWrapper. This is almost
the same as using a mock, but I feel it's much more "light-weight".

The main change is that there is no strict assertion any more for the
number of ReferenceStack::pushInvalidRef() calls. Before this was mixed
into the same array as the valid references, as elements set to "false".
I think the test is as valueable as before without this extra check. If
the rollback stack works or not is already covered by other tests.

Change-Id: I90213557b164b3e43233a3dc393ee3f3d3d556a9
2020-01-20 16:31:48 +01:00
Thiemo Kreuz d8651dda81 Clean up mocks and assertions in ErrorReporterTest
For example, there is no need to create a mock in a callback.

Change-Id: I8879b662ac69ba62fe9c0eb86f592493065e24b1
2020-01-20 14:50:30 +01: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
Thiemo Kreuz b78d85e728 Use StatusValue::isGood() instead of isOK()
The difference between the two is that isOK() only reports "fatals",
while isGood() also reports "warnings" and "errors". I believe we
*want* to report all of these the same way.

Change-Id: I3be832c5db7aba3c03bd2ad8cfbba42362c093fd
2020-01-20 12:35:48 +01:00
jenkins-bot 1ca0905b98 Merge "Fix PHPUnit 8 warning" 2020-01-20 11:06:37 +00:00
Max Semenik 9ecf523eee Fix PHPUnit 8 warning
Bug: T192167
Change-Id: I78ef2aa360e71a5fe214c54807aaa4afbb40c026
2020-01-20 10:33:56 +00:00
jenkins-bot f2cda50778 Merge "Add unit test for section preview regression" 2020-01-20 10:08:43 +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
Thiemo Kreuz ceb3a1ed5f Add unit test for section preview regression
Bug: T242434
Change-Id: I3e87897a1f9f418c4dd72d3137c74340b6646930
2020-01-14 15:10:47 +01:00
jenkins-bot 09f4deede4 Merge "Replace now unused native cloning feature" 2020-01-09 14:13:58 +00:00
jenkins-bot d18fbcffef Merge "Rewrite ReferenceStackTest::provideRollbackRefs for readability" 2020-01-09 12:53:34 +00:00
jenkins-bot ad0c94bf22 Merge "Annotate TODOs with task number" 2020-01-09 12:48:49 +00:00
jenkins-bot 6d02c1569d Merge "Final clean-ups for a more consistent parameter order" 2020-01-09 12:44:55 +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
Thiemo Kreuz 013e1bfa90 Final clean-ups for a more consistent parameter order
* Always have an empty line between @param and @return to improve
readability as well as consistency within this codebase (before, both
styles have been used).

* Flip parameter order in validateRefInReferences() for consistency with
the rest of the code.

* In Cite::guardedRef() the Parser was now the 1st parameter. I changed
all related functions the same way to make the code less surprising.

* Same in CiteUnitTest. This is really just the @dataProvider. But I feel
it's still helpful to have the arguments in the same order everywhere, if
possible.

* Add a few strict type hints.

* It seems the preferred style for PHP7 return types is `… ) : string {`
with a space before the `:`. There is currently no PHPCS sniff for this.
However, I think this codebase should be consistent, one way or the other.

Change-Id: I91d232be727afd26ff20526ab4ef63aa5ba6bacf
2020-01-09 12:13:54 +01: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
jenkins-bot 44f3f5bf44 Merge "build: Updating mediawiki/mediawiki-phan-config to 0.9.0" 2020-01-07 03:47:57 +00: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
libraryupgrader 2e0792a0dd build: Updating mediawiki/mediawiki-phan-config to 0.9.0
One of the most significant changes is when I noticed that the $group
can never be null. We set it to DEFAULT_GROUP before. That's an empty
string.

I'm not very happy with the two @phan-suppress-next-line. Is there a
better way to fix these lines?

Change-Id: I33c1681e2f3857cb6701da71f4ed8893caff4d1e
2019-12-27 19:45:17 +00:00
Thiemo Kreuz ed5d72456d Rewrite ReferenceStackTest::provideRollbackRefs for readability
I hope this is more readable. This patch does two things: It uses
array keys to name all elements in the data provider. (Note these
array keys don't actually do anything, PHPUnit ignores them.) And this
patch merges two parameters into a single $expectedResult.

Change-Id: Ib7adc32bf8bfd523735591d35d0bcabd3b853cfc
2019-12-24 14:31:27 +01: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
Adam Wight 2a3879eafa Harden logic assertions
These should be impossible conditions, we don't want to continue with
processing.

I hate this patch, it's a temporary workaround until someone rewrites
or replaces the rollback logic, for example with a two-pass parse.

Change-Id: I6a1327e397d4272fa412c3f290c2107d867d2854
2019-12-20 14:53:29 +01:00
Thiemo Kreuz 028424a682 More function call argument unpacking
I hope this patch is not to horrifying and can be reviewed. It's
possible to split this into a sequence of smaller patches. Please
tell me.

Change-Id: I4797fcd5612fcffb0df6c29ff575dd05f278bd4d
2019-12-19 12:58:02 +01:00
jenkins-bot 347ad9fb5f Merge "Change order of elements in the refs call stack" 2019-12-19 10:30:35 +00:00
jenkins-bot 45119f8c61 Merge "Move "dir" error handling to validation" 2019-12-19 10:18:24 +00:00
jenkins-bot 0394cd8599 Merge "Add missing @covers tags to tests" 2019-12-19 10:13:56 +00:00
Thiemo Kreuz 38fe3665e5 Change order of elements in the refs call stack
The main benefit is this nifty call: `$this->rollbackRef( ...$call )`

To make this possible, the minimal change I needed to do was to move
the two $argv and $text arguments to the end.

I also tried to order all other arguments as good as I could: Required
first, optional later. Group and name together. Name and extends
together.

All this is private implementation and should not affect anything.

Change-Id: I7af7636c465769aa53122eb40d964eabdd1289ba
2019-12-19 09:27:15 +01:00
Thiemo Kreuz 5c65525c95 Introduce ReferenceStack::appendText
I feel this is a little better than before. It looks like we never need
to *replace* a text that existed before.

This depends on I4a156aa which fixes one of the last remaining trimming
issues. Outside of <references>, a <ref> </ref> with no other content
but some whitespace was already forbidden. But not inside of <references>.
This is relevant for appendText(). It should not be called with null, but
was because of the inconsistent behavior.

Change-Id: I38c9929f2fa6e69482e45919e2f8dbf823cb1c8b
2019-12-19 08:52:48 +01: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
Thiemo Kreuz 92607eecfd Add missing @covers tags to tests
We forgot about these when restructuring the code and introducing these
new methods.

Change-Id: If856a7857f2d50d1dc66a57999d98baa8b924d51
2019-12-18 16:46:15 +00: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
jenkins-bot ce14db4048 Merge "Add parser tests for the responsive="…" feature" 2019-12-18 07:10:22 +00:00
Thiemo Kreuz 1f76199ed8 Add parser tests for the responsive="…" feature
Change-Id: Id9d733dabf82f2c26f51c6fbd1e03fe0574e88a8
2019-12-17 15:51:41 +01:00
Thiemo Kreuz e5cda65fbe Remove single use classes from the use section
The name of the base class in tests is guaranteed to only occur a
single time in a file. There is not much value in making it relative,
and requiring it to appear in the use section. Especially because it
is in the root namespace.

This reflects what I once encoded in the sniff
https://github.com/wmde/WikibaseCodeSniffer/blob/master/Wikibase/Sniffs/Namespaces/FullQualifiedClassNameSniff.php
I wish we could pick this rule and use it in our codebases. But it
seems it is to specific and can't be applied on all codebases, hence
it can't become part of the upstream MediaWiki rule set. At least not
at the moment.

Change-Id: I77c2490c565b7a468c5c944301fc684d20206ec4
2019-12-17 14:57:55 +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 51ff3cc819 Several code cleanups after getting rid of cloning
* Use the Html class to safely create HTML code.
* $this->referenceStack can not be null any more.
* $this->inReferencesGroup is not needed during output, only when
  parsing tags.
* Replace ReferencesStack::getGroupRefs() as well as deleteGroup()
  with a combined popGroup() that does both things.
* Extract the code responsible for the "responsive" behavior to a
  separate function.
* Some TestingAccessWrapper are not needed.

Change-Id: Ie1cf2533d7417ae2f6647664ff1145e37b814a39
2019-12-16 15:47:23 +01:00
Adam Wight 870b9ec181 Rename test to match class
Change-Id: I1814f31ead3882d4c484e0da2808fc7fbc9d2f37
2019-12-12 11:15:13 +01:00
Adam Wight ab07a3253c Remove Parser state from CiteErrorReporter
Finishes breaking the circular reference between Cite and Parser.

This patch also demonstrates how evil it is to allow the error reporter
to be called from anywhere, and have side-effects.  At least it's explicit
now.

Also fixes a bug where the inner error message would not be in the
interface language.

Bug: T240431
Change-Id: Ic3325cafb503e78295d72231ac6da5c121402def
2019-12-12 11:15:07 +01:00
Adam Wight 22b2f78db6 Remove Parser state from ReferencesFormatter
Bug: T240431
Change-Id: I3477a64b9a9dba0cfe890c4b598a51c2f971c76c
2019-12-12 11:12:17 +01:00
Adam Wight 2a5976f007 Remove Parser state from FootnoteMarkFormatter
Bug: T240431
Change-Id: Ie53444114c032e083293d3b5325252debb0640a7
2019-12-12 11:12:17 +01:00
Adam Wight 852a503262 Don't keep parser reference in Cite
This begins our journey of breaking the circular reference between
Cite and Parser.  In later patches the child objects will also take
Parser as a parameter.

Bug: T240431
Change-Id: Ic672bb4bae19ac5f1e1f5817de171d76b3bd8786
2019-12-12 11:12:17 +01:00
Adam Wight a227395e3a Lazy instantiation of Cite
Only create a Cite object if we need one.  Never clearState, just
destroy and recreate later.

This makes it less likely that we leak state between parsers, and
saves memory and processing on pages without references.

It's also preparation to decouple Cite logic from state.

Change-Id: I3db517591f4131c23151c76c223af7419cc00ae9
2019-12-12 11:12:17 +01:00
Thiemo Kreuz 3f2aeb7e31 Rename two Cite… classes and clean up test setups
* All classes are in a Cite\ namespace now. No need to repeat the word
"Cite" all over the place.

* The "key formatter" is more an ID or anchor formatter. The strings it
returns are all used in id="…" attributes, as well as in href="#…" links
to jump to these IDs.

* This patch also removes quite a bunch of callbacks from tests that
don't need to be callbacks.

* I'm also replacing all json_encode().

* To make the test code more readable, I shorten a bunch of variable
names to e.g. $msg. The fact they are mocks is still relevant, and still
visible because these variable names are only used in very short scopes.

Change-Id: I2bd7c731efd815bcdc5d33bccb0c8e280d55bd06
2019-12-12 08:48:02 +01:00
jenkins-bot 9cc9e9cdc1 Merge "Add parser tests for reused extended <ref> before defined" 2019-12-11 16:04:53 +00:00
jenkins-bot 5b7f6d2d35 Merge "Add parser test for duplicate extended references" 2019-12-11 15:35:33 +00:00
jenkins-bot 0dafe64305 Merge "Rename CiteParserTagHooks::initialize to register" 2019-12-11 15:35:32 +00:00
jenkins-bot 5d5fd30a3c Merge "Minor improvements to the test coverage" 2019-12-11 15:35:32 +00: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
jenkins-bot e39b1d6cbd Merge "Use messagelocalizer in CiteErrorReporter" 2019-12-11 11:42:28 +00:00
Thiemo Kreuz d0cb639e03 Minor improvements to the test coverage
We are *so* close to 90%.

This patch should raise the coverage for the CiteDataModule to 100%.
I'm also adding a pure unit test for the clone() behavior. Note the
later is already covered by the CiteDbTest.

Question: Do we want the CiteDbTest to @cover anything?

Change-Id: I40763d01e18991f509bc30b6655aa57b23412fd9
2019-12-11 12:22:59 +01:00
Adam Wight f93f1b4fe0 Use messagelocalizer in CiteErrorReporter
Fixes a bug introduced in Icf61c9a27fd, which would cause a parser
cache split any time the Cite extension was initialized.  The
`setLanguage` interface is regrettable, but I'm hoping it will only
be around temporarily.

Converts an integration test into a unit test and completes coverage.

Bug: T239988
Change-Id: I4b1f8909700845c9fa0cbc1a3de50ee7d42f69a5
2019-12-11 09:53:47 +01:00
jenkins-bot 6556f75c38 Merge "Integration parser test for ParserFunctions" 2019-12-11 08:45:34 +00:00
jenkins-bot 76b4706938 Merge "Rename formatNumNoSeparators() to localizeDigits()" 2019-12-11 08:29:09 +00: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
Adam Wight cad4d18458 Integration test to hit cloned Cite bug
Tickle very particular edge case in which a recursive parse corrupts
the $parser->extCite object.

Bug: T240248
Change-Id: I70d100e88fa72825194ed9c477b030bbf0b6b486
2019-12-10 17:07:44 +01:00
Thiemo Kreuz 75016551e7 Rename formatNumNoSeparators() to localizeDigits()
Because that is what it does. Note our method is different from the one
in the Language class. We only accept strings.

Change-Id: I39107e837cc29f2d7c8867c1e602aa643f9e1a57
2019-12-10 16:21:12 +01:00
Thiemo Kreuz 01bcfa773d Rename CiteParserTagHooks::initialize to register
It's called "register" in MediaWiki core as well.

Change-Id: Iad3dc3badbb7ad10a14276c3a144376acf70e5e5
2019-12-10 14:19:33 +00:00
Thiemo Kreuz 71c6dc7fe4 Better naming for ReferenceFormatter class and methods
This class renders a <references> tag and everything inside. The
previous name sounds like it is responsible for rendering the contents
of a <ref>…</ref> tag. I mean, the class contains a method that does
exactly this. But this method is private.

Change-Id: I1cd06c9a11e0a74104f2874a34efa3e0843a0f70
2019-12-10 08:40:09 +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
jenkins-bot 4a0026d9bc Merge "Split validation function depending on inReferencesGroup" 2019-12-09 12:21:17 +00:00
Adam Wight 8097c4c148 Split validation function depending on inReferencesGroup
Some validation is exclusively used in a specific context, some is shared.

Change-Id: I390db1c9d4854871e25a2e74411476e4e1c0b66f
2019-12-09 12:27:52 +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 a8e882e39f Merge "Show "Preview" headline in user instead of content language" 2019-12-09 10:13:17 +00: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
Thiemo Kreuz 8fdce945bd Show "Preview" headline in user instead of content language
This partly reverts Id7a4036e64920acdeccb4dfcf6bef31d0e5657ab.

The message "cite_section_preview_references" says "Preview of references".
This line is not meant to be part of the content, but an interface message.
It should use the users (interface) language, not the content language.

Change-Id: I1b1b5106266606eb0dfaa31f4abd3cee9ba92e8c
2019-12-09 10:53:07 +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
Adam Wight 3d80501829 Narrow message localizer interface
We never access Language directly, so proxy its method instead of
returning the full object.

I believe I've found a bug, but not fixing here: the footnote body
numeric backlinks like "2.1" behave as if they were decimals rather
than two numbers stuck together with a dot.  So they are localized
to "2,1".

Bug: T239725
Change-Id: If386bf96d48cb95c0a287a02bedfe984941efe30
2019-12-06 12:17:09 +01:00
jenkins-bot f745a6b2f2 Merge "Roll up a range in test fixture" 2019-12-05 14:50:29 +00:00
Adam Wight 01c76f46a6 Use message localizer in CiteKeyFormatter
Makes more tests easier.

Change-Id: I222ba61bfcf0be3e29cb04e39f44f0be7a9e0778
2019-12-05 14:57:32 +01:00
Adam Wight 1ce4079ce2 Use message localizer in FootnoteMarkFormatter
Completes test coverage.

Change-Id: Ib2ec24cf4a9de52769744d1888cb13d2bf08ae3b
2019-12-05 14:56:53 +01:00
Adam Wight 430086cb6b Use the message localizer in Cite
Allows us to convert another integration test into a unit test.

Change-Id: Id7a4036e64920acdeccb4dfcf6bef31d0e5657ab
2019-12-05 13:23:31 +01:00
Adam Wight 4dec6afbb8 Tests for checkRefsNoReferences
One of these cannot be a unit test yet, because of a wfMessage call.

Change-Id: Ie2e2e28ee4c369cc7380c528411665fbb51691be
2019-12-05 09:14:30 +01:00
Adam Wight 855a3d6d48 Roll up a range in test fixture
This... might not be helpful.

Change-Id: Iff3342d5fbf81aa27b9aaaf1b3fc4c59fa65a365
2019-12-05 09:05:03 +01:00
Adam Wight 1f92841662 Test coverage for guardedReferences
Change-Id: Id97ba7a965dfd78579fc18e7f3d21a595e6bf432
2019-12-05 08:59:22 +01:00
jenkins-bot 336dd4a27c Merge "Complete validateRef coverage" 2019-12-04 17:18:40 +00:00
jenkins-bot 6386212f1e Merge "Tests for guardedRef" 2019-12-04 17:09:19 +00:00
Adam Wight 5705228d17 Complete validateRef coverage
Change-Id: Id61fba34a8815a0c512ecf4bc57da3be4e15c8bb
2019-12-04 18:00:13 +01:00
Adam Wight 3d049159c2 Tests for guardedRef
This is a mess of a function, and the tests show it.  There are lots
of side-effects and context-sensitivity, which can be addressed in
later work.  The interface with ReferenceStack is too wide.

Change-Id: I00cab2a555b2a9efd32d937979cd722d43ac1005
2019-12-04 16:06:10 +00:00
Thiemo Kreuz a7c4e14f42 Remove obsolete ParserBeforeTidy hook handler
I was able to track this code down to I093d85d from 2012, which was done
right after the ParserAfterParse hook was introduced. I believe the
redundant code path was left to keep the Cite extension compatible with
old MediaWiki versions that did not had this hook yet.

I also noticed this code path is most probably entirely redundant with
the current version of MediaWiki. The *only* thing this code does is
blocking the ParserBeforeTidy hook from doing the same thing a second
time if the ParserAfterParse hook was called before. But it does *not*
block any other compination, e.g. if the two hooks are called the other
way around, or the same hook twice.

In core, it looks like it is impossible for the ParserBeforeTidy hook
being fired without the ParserAfterParse hook being fired before. If this
is true, this is in fact dead code.

Change-Id: Iacf8b600c7abdeaf89c22c2fc31e646f57245e47
2019-12-04 16:56:43 +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
Adam Wight 484373c21e Complete tests for FootnoteBodyFormatter
Change-Id: I7cfa38f59d00be30345eb5a200f62e17197d2c76
2019-12-04 13:48:15 +01:00
Adam Wight c09d90aff3 Use message localizer in FootnoteBodyFormatter
Makes the class more easily testable.

Patch also changes an integration test into a unit test.

Change-Id: I545730404aceed7e3857d96f4fd3c1b0a900c0c2
2019-12-04 13:41:44 +01:00
Adam Wight bccb92335f Introduce ReferenceMessageLocalizer
Encapsulate the language interfaces, this will be used to replace
global wfMessage calls in future patches.

Change-Id: I7857f3e5154626e0b29977610b81103d91615f65
2019-12-04 13:40:05 +01:00
Adam Wight 817c58230f Rename test to follow function name
Change-Id: I6e925f65ba7c59738ad4e55748f1efdb4cf04573
2019-12-04 11:23:55 +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 d40bf92396 Make integration test into a unit test
We were mocking ParserOutput anyway, so this is the same test as before.

Change-Id: If31898b537db946b6b4a595663d3894d05d94e77
2019-12-03 14:07:48 +01:00
Adam Wight 96db7944eb Cover rollback with tests
Fixed an unsafe array access during rollback.

Change-Id: Id9ee8976e3bae24501c18abf462e3e19894caff0
2019-12-03 09:53:23 +01:00
Adam Wight 4cf8faea48 Cover edge cases with unit tests
Change-Id: If715788292631f2e1f4cf970c1ae7c7fd7d514e1
2019-12-03 09:53:23 +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 221e657dfe Merge "Formatter takes responsibility for rendering footnote mark" 2019-12-02 15:32:05 +00: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