Commit graph

251 commits

Author SHA1 Message Date
Adam Wight 76e6e870d4 Include more information in missing parent placeholder
This allows the subreferences to be collected together under a heading.

Bug: T353451
Change-Id: Ibf28f0baca14de8140c87b03ad4aa86d2f81a20d
2024-01-05 11:21:12 +00:00
Adam Wight 5a69c54900 Render list-defined parent without a backlink
In this case, there was never a ref with this name in the article so
no backlinks should be rendered.

TODO:
* test case with empty parent backlink and LDR parent

Bug: T353451
Change-Id: I8a7abd05a48ce83da3beb92b15e894d53252bd33
2024-01-05 12:07:22 +01:00
jenkins-bot 23a1a8999d Merge "Remove test for a private method" 2024-01-04 16:30:29 +00:00
Adam Wight ddf5cb2458 Remove test for a private method
Testing internal methods is brittle.  This code path is already
covered by parser test "Valid follow="…" after it's parent"

Bug: T353451
Change-Id: I3b7a4b9962de1f25a7b57f82d80813219d633594
2024-01-04 17:07:36 +01:00
jenkins-bot 733824005a Merge "Drop unused cite_reference(s)_link_prefix messages" 2024-01-04 16:04:34 +00:00
jenkins-bot ab20cb3cdf Merge "Rename appendText() to resolveFollow()" 2024-01-04 15:29:44 +00:00
thiemowmde ddda536792 Drop unused cite_reference(s)_link_prefix messages
Same as Icfa8215 where we removed the …_suffix messages.

This patch is not blocked on anything according to CodeSearch:
https://codesearch.wmcloud.org/search/?q=cite_references%3F_link_prefix

According to GlobalSearch there are 2 usages we need to talk about:
https://global-search.toolforge.org/?q=.&regex=1&namespaces=8&title=Cite.references%3F.link.prefix.*

zh.wiktionary replaces "cite_ref-" with "_ref-", and "cite_note-"
with "_note-", i.e. they did nothing but remove the word "cite". This
happened in 2006, with no explanation.

ka.wikibooks and ka.wikiquote replace "cite_note-" with "_შენიშვნა-",
which translates back to "_note-". One user did this in 2007,
16 seconds apart.

It appears like both are attempts to localize what can be localized,
no matter if it's really necessary or not.
https://zh.wiktionary.org/wiki/Special:Contributions/Shibo77?offset=20060510
https://ka.wikiquote.org/wiki/Special:Contributions/Trulala?offset=20070219
Note how one user experimented with an "a" in some of the edits to
see what effect the change might have, to imediatelly revert it.

The modifications don't really have an effect on anything, except on
the anchors in the resulting <a href="#_ref-5"> and <sup id="_ref-5">
HTML. It might also be briefly visible in the browser's address bar
when such a link is clicked. We can only assume the two users did this
to make the URL appear shorter (?). A discussion apparently never
happened. Bot users are inactive.

Both pieces of HTML are generated in the Cite code. Removing the
messages will change all places the same time. All links will
continue to work. The only possible effect is that hard-coded
weblinks to an individual reference will link to the top of the
article instead. But:
a) This is extremely unlikely to happen. There is no reason to link
   to a reference from outside of the article.
b) Such links are not guaranteed to work anyway as they can break
   for a multitude of other reasons, e.g. the <ref> being renamed,
   removed, or replaced.
c) Even if such a link breaks, it still links to the correct article.

There is also no on-wiki code on zh.wiktionary that would do anything
with the shortened prefix:
https://zh.wiktionary.org/w/index.php?search=insource%3A%2F_%28ref%7Cnote%29-%2F&title=Special%3A%E6%90%9C%E7%B4%A2&profile=advanced&fulltext=1&ns2=1&ns4=1&ns8=1&ns10=1&ns12=1&ns828=1&ns2300=1

I argue this is safe to remove, even without contacting the mentioned
communities first.

Bug: T321217
Change-Id: I160a119710dc35679dbdc2f39ddf453dbd5a5dfa
2024-01-04 13:17:42 +01:00
thiemowmde ca3203699c Capitalized dir="RTL" should not trigger any error
This fixes a minor issue introduced in I294b59f. Two identical
dir="…" with different capitalizations should not be reported as an
error.

Turns out the implementation in the Cite extension doesn't care
about this capitalization at all. That's why I suggest to do the
normalization as early as possible. This is slightly different in
the Parsoid implementation.

Bug: T202593
Change-Id: I96b4a281d6020d61d1f36ec027cf833bbb244f03
2024-01-03 16:30:16 +00:00
Adam Wight d2b92c5253 Explicit test fixture field names
Bug: T353451
Change-Id: I8a308dd2785939da52a698cf5e63bce4bc228b77
2023-12-22 23:52:22 +01:00
Adam Wight 5d1335e279 Explicit parameter names for all test fixtures
This is much more readable.  Patch changes nothing.

Bug: T353451
Change-Id: I72b58881a7329dbe98659553b84e53896ccafc2b
2023-12-21 20:59:25 +01:00
Adam Wight e0e8b2dc33 Drop constant parameters
Some tests didn't vary the key parameter, so converting to a
constant.

Change-Id: I9a42352573cc16fec11a799b878c06751eb03fc8
2023-12-21 20:52:43 +01:00
thiemowmde d73a76dce6 Rename appendText() to resolveFollow()
There is only 1 user left after Icf16965.

Bug: T353266
Change-Id: I4cafdcbe0a23dd7950613a385cb552e7a84e7f26
2023-12-19 14:49:52 +01:00
jenkins-bot 9b87bc717d Merge "Various cleanups to PHPUnit test mock setup" 2023-12-18 12:36:11 +00:00
thiemowmde 742a9ffbf5 Track warnings separately in ReferenceStack
Check out how this gets rid of so many "to do" as well as
"deprecated" comments.

Next qustion: The elements in the stack become more and more
complicated. It's probably worth converting them from arrays into
first-class objects. But this is for another patch.

Bug: T353266
Change-Id: If14acd1070617ca8c4d15be6b1759bd47ead4926
2023-12-15 16:41:04 +01:00
xiplus f7a181ed42 Give a different error from too_many_keys when 'follow' attribute conflicts
Add message "cite_error_ref_follow_conflicts" for tags with
conflicting parameters.

Bug: T299280
Change-Id: Ie64f4ab4831966f66f812ea67cc244718f818afb
2023-12-15 15:23:53 +01:00
thiemowmde 9304e24551 Various cleanups to PHPUnit test mock setup
For example, use convenient upstream methods, and generally make the
test setup a bit more readable.

Bug: T353227
Change-Id: Ifab71041fcc3f804315793ca7b783f84829c7a0f
2023-12-15 11:45:35 +00:00
thiemowmde 4377f0923d More simple and consistent @covers and @license tags
Same arguments as in Iafa2412. The one reason to use more detailled
per-method @covers annotations is to avoid "accidental coverage"
where code is marked as being covered by tests that don't assert
anything that would be meaningful for this code. This is especially a
problem with older, bigger classes with lots of side effects.

But all the new classes we introduced over the years are small, with
predictable, local effects.

That's also why we keep the more detailled @covers annotations for
the original Cite class.

Bug: T353227
Bug: T353269
Change-Id: I69850f4d740d8ad5a7c2368b9068dc91e47cc797
2023-12-15 12:12:16 +01:00
thiemowmde d0d5fbbee6 Add temporary ErrorReporter::firstError helper function
I hope this makes other refactorings a little easier.

Bug: T353266
Change-Id: Ib574d4d54ba2c8bc1310822539336ad71c4309ef
2023-12-14 17:16:49 +01:00
thiemowmde 01dcfbac47 Move Validator tests to a separate class
I wanted to make this a unit test but it turns out the
Sanitizer::safeEncodeAttribute() calls currently make this
impossible.

Bug: T353269
Change-Id: I5266e7b8b67db1c812dc9e4675d0c079ab1f9a40
2023-12-14 15:51:26 +00:00
jenkins-bot 6fc8ee7fec Merge "Get rid of "guarded <references>" terminology" 2023-12-14 14:25:57 +00:00
jenkins-bot 78b40a8c6b Merge "Extract validation to a separate class" 2023-12-14 14:18:40 +00:00
thiemowmde c794962df7 Use short fn() syntax in tests where it makes sense
We can use this syntax now. It was introduced in PHP 7.4.

Bug: T353269
Change-Id: I5404b33b654efb01171fa2b4ad3925170ffd0e56
2023-12-14 08:05:01 +00:00
jenkins-bot 5b4e869014 Merge "tests: Widen @covers annotations" 2023-12-14 07:57:46 +00:00
thiemowmde 12c7ad7504 Get rid of "guarded <references>" terminology
This patch only moves existing code around without changing any
behavior. What I basically did was merging the old "guardedReferences"
method into "references", and then splitting the resulting code in
other ways. Now we see a few other concepts emerging. But the idea
something would be "guarded" (how?) is gone.

The most critical detail in this patch are the new method names, and
how the code is split. The names should tell a story, and the methods
should do exactly what the name says. Suggestions?

Bug: T353266
Change-Id: I8b7921ce24487e9657e4193ea6a2e3e7d7b0b1c3
2023-12-14 08:44:40 +01:00
thiemowmde a6a0f66130 Extract validation to a separate class
This removes almost 200 lines from the main class.

This patch intentionally doesn't make any changes to the code but
only moves it around. Further improvements are for later patches.

Bug: T353269
Change-Id: Ic73f1b7458b3f7b7b89806a88a1111161e3cf094
2023-12-14 07:43:29 +00:00
jenkins-bot bf53249893 Merge "Move a bit of code out of Cite::guardedReferences" 2023-12-14 02:10:06 +00:00
Timo Tijhof 2ff327df53 tests: Widen @covers annotations
> We lose useful coverage and spend valuable time keeping these tags
> accurate through refactors (or worse, forget to do so).
>
> I am not disabling the "only track coverage of specified subject"
> benefits, nor am I claiming coverage in in classes outside the
> subject under test.
>
> Tracking tiny per-method details wastes time in keeping tags
> in sync during refactors, and time to realize (and fix) when people
> inevitably don't keep them in sync, and time lost in finding
> uncovered code to write tests for only to realize it was already
> covered but "not yet claimed".

https://gerrit.wikimedia.org/r/q/owner:Krinkle+is:merged+message:%2522Widen%2522

Change-Id: Iafa241210b81ba1cbfee74e3920fb044c86d09fc
2023-12-14 01:54:48 +00:00
thiemowmde 04208b5fd1 Remove PHPDocs that just repeat what the code already says
We removed a bunch of now redundant docs already, see e.g. Ie0692fa.

Change-Id: I55c62d935bdec8bedaebc2666fca3eb17309b0c7
2023-12-13 12:44:41 +01:00
jenkins-bot c3aa27f2a1 Merge "Avoid a few isset() in favor of more recent syntax" 2023-12-12 23:58:45 +00:00
thiemowmde 689bafdd7f Use upstream assertStatusError and such in tests
The main benefit is that these methods give good debug output in
case they fail.

Bug: T353266
Change-Id: I0423737240c35c18078863a7eb1d8e4779363973
2023-12-12 19:16:50 +01:00
thiemowmde 9425bb3248 Move a bit of code out of Cite::guardedReferences
The main benefit is that the two lines that set and reset
$this->inReferencesGroup are now next to each other. More can be
done in later patches.

Bug: T353266
Change-Id: Ib3f40c40e0b1854f8e5a32af600f28931fffdb8c
2023-12-12 18:06:58 +00:00
thiemowmde fee8606db6 Avoid a few isset() in favor of more recent syntax
As well as replacing a few `=== null` comparisons with the new ??=
operator.

Bug: T353227
Change-Id: I5b273f452d1e46d37fc28861b54c4e1f19a7a65a
2023-12-12 12:13:42 +00:00
jenkins-bot 34798cce42 Merge "Change all tests to use overrideConfigValue" 2023-12-12 08:59:46 +00:00
thiemowmde 44ba7a89e2 Parse error messages as late as possible
This moves the actual parsing down to be done much later in the
process. This won't make any difference in production but makes it
easier to refactor the code further.

Note I tried to use a StatusValue object but couldn't because it
merges seemingly identical messages, while the plain array is fine
with containing duplicates. There is one parser test that covers
this. While we could change this it needs discussion and most
probably a PM decision.

Change-Id: I7390b688a33dace95753470a927bbe4de43ea03a
2023-12-11 18:28:35 +00:00
thiemowmde 696c35f496 Change all tests to use overrideConfigValue
Two problems:

1. Manipulating globals directly affects all following tests. They
are not independent from each other. This problem can be seen in
CiteTest.

2. Some test cases in testValidateRef don't test what you think.
For example, the test for a conflicting "extends" + "follow" was not
failing because of the conflict but because "extends" was disabled
and disallowed.

Change-Id: Iaa4e1f3f3222155d59984e577cba3f0b8dec40c3
2023-12-11 12:17:15 +01:00
Umherirrender c9773965ca Use namespaced classes
Done automatically via script

Change-Id: I40d64a194ad420c75dfa85711c53e35586895929
2023-12-10 23:18:51 +01:00
thiemowmde 202c0d3636 Drop unused …_suffix and …_key_with_num messages
The three messages cite_reference_link_key_with_num,
cite_reference_link_suffix, and cite_references_link_suffix are not
used for anything.

According to CodeSearch:
https://codesearch.wmcloud.org/search/?i=1&q=cite_references?_link_(key|suffix)

According to GlobalSearch:
https://global-search.toolforge.org/?q=.&regex=1&namespaces=8&title=Cite.references?.link.(key|suffix).*
For comparison:
https://global-search.toolforge.org/?q=.&regex=1&namespaces=8&title=Cite.references?.link.prefix.*

They are not meant to be localized, as noted in qqq.json. As many
messages in Cite the idea is that individual wikis can customize the
generated HTML (!) via such messages. These particular ones apparently
have been introduced just because it's technically possible, but never
been used for anything. They exist since the very first commit from
2005: https://phabricator.wikimedia.org/rECITb714bf09

Note how these messages aren't even visible anywhere, except in the
browser's address bar as part of a #… fragment.

This obviously doesn't solve T321217 but helps minimizing the
surface.

Bug: T321217
Change-Id: Icfa82155e3b02df39bb6e924bc472f6edc565d5f
2023-12-08 09:26:05 +01:00
jenkins-bot f455dd3f79 Merge "Re-arrange code in preparation for T298278" 2023-12-05 14:53:42 +00:00
jenkins-bot 1e5cd295e0 Merge "Split off separate key normalization function" 2023-12-04 12:15:21 +00:00
thiemowmde 5f5e9ec9f0 Re-arrange code in preparation for T298278
This patch makes only sense together with I5a64ac4 where it is split
from. See I5a64ac4 for details.

The idea is that this patch just re-arranges the code without making
any changes to how the code behaves. This leaves a minimal change
behind that's much easier to revert, if needed.

Bug: T298278
Change-Id: Ie78313b7f3ac1ec7bce5ac7512e60a3bb011480a
2023-12-04 08:29:53 +01:00
thiemowmde 858fdcefd9 Split off separate key normalization function
This patch does two things:

1. The "normalization" function was never only doing normalization,
but also all the necessary HTML encoding. This is now more visible
and split into two separate functions.

2. To make this easier we change the order slightly. Because of this
the normalization step must now consider spaces. Before spaces have
been converted to underscores by escapeIdForLink.

The results are all the exact same as before.

This is split from I5a64ac4 to make that easier to review.

Bug: T298278
Change-Id: I9435a2ddaa21559e29587c58b7523103141467f7
2023-11-30 09:43:35 +01:00
gerritbot 3d34307f87 Update StaticUserOptionsLookup's FQN
User-options related classes are being moved to
the MediaWiki\User\Options namespace in MediaWiki Core;
reflect that change here.

Bug: T352284
Depends-On: I42653491c19dde5de99e0661770e2c81df5d7e84
Change-Id: I22ff2effcf9b7f2162f5d57608d8ec3651b48dd7
2023-11-29 17:55:07 +00:00
Daimona Eaytoy c71109b97b Update tests for PHPUnit 9.6
- Avoid withConsecutive()

Bug: T342110
Change-Id: Icb951b461c5e6ea1ced2ab8a183c53265d4a2b4f
2023-11-22 00:15:20 +01:00
Fomafix 96fad3ea99 tests: Use $this->getServiceContainer()
Use
	$this->getServiceContainer()
instead of
	MediaWikiServices::getInstance()
in tests.

Change-Id: I5e56524b85ba6e34cecffba2f24fb44b3f66ec8f
2023-10-26 19:59:14 +00:00
gerritbot 198ae5b21c Replace some moved Title class uses, now MediaWiki\Title\Title
Bug: T321681
Change-Id: Idde5511e075fcec40cad12c7369abec3ee4d096c
2023-08-19 04:13:55 +00:00
Daimona Eaytoy e8da022968 Mark CiteDbTest as using the page table
The test creates the Cite-tracking-category-cite-error page.

Change-Id: I20b2007b943afd69bef8fcce18f382e64d752c57
2023-08-11 17:29:33 +02:00
jenkins-bot 0fc08a2ac7 Merge "Replace extremely slow parser test with fast unit tests" 2023-07-28 01:05:39 +00:00
thiemowmde 5aa6cb0c7b Replace extremely slow parser test with fast unit tests
This parser test is a bit obscure, in my opinion. We added it in
I8c4de96 to make sure we don't get thousand separators in most
places.

We continued reworking the code since then. By now it's effectively
impossible to "accidentally" get thousand separators. The
problematic methods from the Language class are not even accessible
any more from this code.

To make the tests more robust we now use createNoOpMock (done via
the previous patch) where it matters, specifically for all Language
and Parser mocks. This proves the problematic Language methods are
never called.

Bug: T253743
Bug: T238187
Change-Id: I9bfe1f4decfaf699996da63e19473c2c0d581d9d
2023-07-28 00:32:38 +00:00
jenkins-bot 51308066fa Merge "Remove unused private method from CiteDbTest" 2023-07-27 19:02:16 +00:00
jenkins-bot ebd355c067 Merge "Replace all Language and Parser mocks with no-op mocks" 2023-07-27 18:59:53 +00:00