Commit graph

228 commits

Author SHA1 Message Date
Adam Wight 1af67c0416 Prematurely move References test to integration
This isn't needed until patch I576dfff415, but is split into a
predecessor to keep the test and logic changes obviously separate.

Bug: T377454
Change-Id: Iff4a96be77af53a71a1ebe179f31bcc214182bfe
2024-10-31 11:16:33 +00:00
Adam Wight 60fbb32c1a Split apart mark formatting into a simpler labeler
The new mark label renderer is a service and can be called from
Parsoid.

Bug: T377454
Change-Id: I6f4983c4288bf29954ad4e5fa3309f9bdf48215d
2024-10-31 11:16:13 +00:00
Andre Klapper 41bafe6f2c Use explicit nullable type on parameter arguments (for PHP 8.4)
Implicitly marking parameter $... as nullable is deprecated in PHP
8.4. The explicit nullable type must be used instead.

Bug: T376276
Change-Id: I73a4ce1ecd9b4fe040e5bfd22889e783071fab0d
2024-10-26 14:56:31 +02:00
Umherirrender 411ee7efad Use namespaced classes
Changes to the use statements done automatically via script

Change-Id: I0768f296b528c81edd6bd9ac7d76515b15f3da6f
2024-10-19 23:25:16 +02:00
Fomafix 521f44ede5 Inject MainConfig services into Cite
Change-Id: I24cc3e4965d4a9fe41dcd799dac4cbce762fb068
2024-09-28 22:01:09 +00:00
jenkins-bot 3b197da578 Merge "Use dependency injection for References/RefProcessor" 2024-09-25 17:18:42 +00:00
C. Scott Ananian 7df30e0651 Use dependency injection for References/RefProcessor
This avoids the use of Parsoid's SiteConfig::getMWConfigValue() method,
which is unnecessary when the extension has direct access to MediaWiki
services itself.

This also fixes the omission of CiteResponsiveReferencesThreshold from
the extension.json.

Change-Id: I01b43136b0827f185523f1318253372b09750de4
2024-09-12 11:57:28 -04:00
Adam Wight c7b60735fe Move Reference Previews user preference into the Cite extension
This seems to play well with Popups with and without
Ie8fa1672b9fd .  However, it's not clear to me why this still works
and even gives priority to the Popups implementation when present,
regardless of the order the extensions are loaded in.  Happily, this
is the desired behavior.

Bug: T363162
Change-Id: Ic479c0a381ee16d1abcecfdd5ee48f0afccc1d3f
2024-09-10 15:33:50 +02:00
Adam Wight 8e4c83bfc6 [refactor] Rewrite context as service
Puts the ReferencePreviewsContext behind a service to improve
testability.

Bug: T363162
Change-Id: I63aa4144081a2625da1b89f4d56adb612741c8fc
2024-09-06 16:26:20 +02:00
jenkins-bot 1116cfb9a9 Merge "Allow disabling the alternative backlink labels message" 2024-09-02 19:34:57 +00:00
jenkins-bot 72056c5fd0 Merge "Make use of createNoOpMock in ReferenceListFormatterTest" 2024-08-29 10:21:44 +00:00
jenkins-bot 56ccdd0e48 Merge "Move existing ReferenceListFormatter code closer together" 2024-08-27 09:40:50 +00:00
thiemowmde b223649f9d Move existing ReferenceListFormatter code closer together
This moves existing code around a little bit without changing
anything, as proven by the tests.

This is motivated by Iaca0e14.

Change-Id: I30366d32b07f87f238b045f0d7817686b5cc26bd
2024-08-13 12:42:02 +00:00
thiemowmde a4ffedbcc2 Remove not needed int type from ReferenceListFormatter
It's always a string anyway. Also improve the test coverage. The
localization was pretty much untested before.

Change-Id: Ie6f34c67ae7dd9559346eb45a2604e14c5633991
2024-08-13 10:46:52 +02:00
thiemowmde 3cebd437e8 Make use of createNoOpMock in ReferenceListFormatterTest
The main benefit is that this makes it more obvious what is actually
under test, and what's not.

This is mainly motivated by the ongoing work in Iaca0e14.

Change-Id: Icbf1b824ba1f5468dbdb30445134db2b568e19f8
2024-08-13 10:41:04 +02:00
thiemowmde d885a0c6b5 Allow disabling the alternative backlink labels message
The message cite_references_link_many_format_backlink_labels contains
e.g. "a b c …" and so on, to be used as alternative backlink labels
when a single reference is re-used multiple times. The default numeric
rendering is "1.1 1.2 …" and so on.

The two labels end in the message cite_references_link_many_format as
parameters $2 and $3. But only one of the two parameters is ever used.
By default the alternative label in $3 is unused.

This implies that everything about these alternative backlink labels
including the error message is effectively dead code most of the time,
never to be seen on the majority of wikis.

This change makes it possible to disable the message without breaking
anything. Instead the code will silently fallback to the default
behavior of showing the numeric label. This is much more efficient
than rendering possibly hundreds of error messages that never show
up anyhere. The same optimization is already done for the extremely
similar cite_link_label_group-… messages, see
FootnoteMarkFormatter::fetchCustomizedLinkLabel.

This is also partly motivated by T335129 as well as T321217.

Change-Id: Iab818d7db7eddaf7091234f6a22a18cdff70f8e8
2024-08-12 13:36:20 +02:00
jenkins-bot d056400800 Merge "Simplify ErrorReporter mocks in tests" 2024-08-12 09:17:19 +00:00
jenkins-bot 7955321a2f Merge "Remove unused test setup from ReferenceListFormatterTest" 2024-08-12 09:02:24 +00:00
thiemowmde fb8494b4dd Relax support for error/warning message keys with dashes
This is a direct follow up for I6f05842 where we already started
supporting dashes, but converted them to underscores. The only change
in this patch is that the CSS class will use the message key as it
is, without the dashes being converted to underscores. I added a test
case specifically for this.

Bug: T352676
Change-Id: Ic22e897c27b8371e3b1ed63932b619e7af71bd5c
2024-08-09 12:29:34 +00:00
thiemowmde f3b732bf93 Simplify ErrorReporter mocks in tests
In some tests we want to see the message parameters. But not here.
Simply echoing the message key (thats parameter number one) is
enough.

Change-Id: Id9824cbbe944c84c9fd1932b0863ac1b3f232b75
2024-08-07 18:22:59 +00:00
thiemowmde e078155e94 Remove unused test setup from ReferenceListFormatterTest
I think this was just a mistake in I5457304 when this test was
written. There was never an ->exists() call in the code, as far as
I can see.

This is motivated by our ongoing, probably year-long efforts to
clean this codebase up, see T335129.

Change-Id: I72d89213c5cff06d78ac119b3c79827afbd0b4f5
2024-08-05 16:25:03 +02:00
Umherirrender 3de8f65114 Use namespaced classes
Changes to the use statements done automatically via script

Change-Id: I4b49c4ab87f1fcd5cdf29d5f882dec3c3902e146
2024-06-08 23:31:42 +02:00
jenkins-bot 4f8e06b97b Merge "Make it possible to disable the "cite_error" wrapper message" 2024-05-27 08:16:04 +00:00
C. Scott Ananian fc5f22b32e CiteParserHooksTest: make test compatible with removal of dynamic property
Whether the dynamic property is present or not, it should have a null
value when 'unset' -- and don't use `unset` to delete an *actual*
property when one is present!

Change-Id: Ifcb9492cc5c814d702c6e61e8231abfd8ea0647c
2024-04-12 15:23:14 -04:00
jenkins-bot 1fdc65548b Merge "Add strictly typed RefGroupItem class to Parsoid code" 2024-04-01 03:05:52 +00:00
jenkins-bot 51e569d888 Merge "Allow visualeditor-cite-tool-name-… messages to be missing" 2024-03-28 15:27:14 +00:00
thiemowmde 9e1c4645be Add strictly typed RefGroupItem class to Parsoid code
This does the exact same as the previously used generic stdClass
object, just strictly typed. Turned out to be surprisingly
straightforward, as proven by the small size of this patch.

I'm intentionally not adding anything new in this patch. For
example, the new class is perfect to write longer documentation
for every field. But this is for a later patch.

Change-Id: Ibf696f6b5ef1bfdbe846b571fb7e9ded96693351
2024-03-14 11:59:34 +01:00
thiemowmde 650d2c296d Add basic PHPUnit tests for Parsoid implementation
There is much more to test, but it's a start.

Intentionally build as pure unit tests to make them as fast as
possible.

Bug: T354215
Bug: T358652
Change-Id: Iae1a8086b8f2b9e5b11e0117bd3f19fdaa087df0
2024-03-12 09:06:16 +00:00
thiemowmde 2b32f15c8c Allow visualeditor-cite-tool-name-… messages to be missing
The information read from the …cite-tool-definition.json files is
effectively user input, even if only interface administrators can
edit it. Usually we carefully validate user input. But as of now
this code starts failing with all kinds of uncatched errors.
* An entry with no name, an empty name, or a name that's not a
  string will cause all kinds of undefined behavior.
* An entry with an empty title results in an invisible button.
* A missing message results in a technical <…> placeholder, even if
  the name is usually a sensible fallback.

Note that hard-coding titles as plain text strings in the ….json
file was already possible.

Change-Id: Iddcedbe859e86ac4c3f79a53d36237daff86c0db
2024-03-06 14:21:08 +01:00
jenkins-bot 46499cd9c0 Merge "Rename ReferencesFormatter to ReferenceListFormatter" 2024-02-07 18:00:04 +00:00
thiemowmde 7c75d44b8a Rename ReferencesFormatter to ReferenceListFormatter
I always found the name a little ambiguous. The fact that it outputs
an actual HTML list and not just some "references" – whatever that
means – is relevant, in my opinion.

Change-Id: I0d169455c8d2b42d62da4dccb8376c09fb6902bc
2024-02-07 18:20:02 +01:00
thiemowmde 802b495160 Make it possible to disable the "cite_error" wrapper message
… as well as "cite_warning". Both are extremely trivial and don't
really do anything by default. All they do is to add the prefix
"Cite error:" or "Cite warning:" to all error messages.

This patch will make it possible to disable both messages by
default, i.e. replace their default in en.json with "-" without
breaking anything. That's part of the plan outlined in T353695.
Local on-wiki overrides will continue to work.

Bug: T353695
Change-Id: I374800d0d0b837cd17ed3a1fdde20b70325b06de
2024-02-01 17:36:27 +01:00
thiemowmde 5eea8ebd90 Simplify ErrorReporter PHPUnit test setup
This was slightly overengineered ever since I4b1f890 and slowly became
more and more complicated over time, notably when withConsecutive was
replaced in Icb951b4. Turns out this was never really needed. It's
impossible to get more than one tracking category from this code path.

While we might add more tracking categories later that will most
probably not happen in this code path.

Change-Id: Ie32d17bac8d3518c985b18f83a846c3fb2bd053f
2024-02-01 17:28:44 +01:00
Adam Wight c36d1b90a5 Move extendsCount into parent ref item
Eliminates a complex shared structure in favor of more encapsulation.

Change-Id: I70efabf0ee263ac578472e16dc35047b0601b7ff
2024-01-10 11:55:16 +01:00
Adam Wight 0e01e39061 Encapsulate ref: groupRefs returned as objects
This patch only affects the consumers of groupRefs.

Bug: T353451
Change-Id: I1eff735dbc26dda07aa8ac7af9ea4ddc0906f5a4
2024-01-09 10:22:04 +01:00
Adam Wight f148c65078 Encapsulate ref: pushRef returns an object
This patch affects a few methods which use the output of pushRef.

Bug: T353451
Change-Id: I10b3fe89406c11cdaede92f18a4b96586ecaf5a0
2024-01-09 10:18:57 +01:00
Adam Wight 262fbe24eb Encapsulate ref object: limited to ReferenceStack
This encapsulation gives us field name, type validation and code
documentation.

This patch only affects ReferenceStack and continues to return
approximately the same array outputs to callers.  Some additional
information is included and the placeholder column has a new name.

Bug: T353451
Change-Id: I405fe7ac241f6991fd4c526bfbb58fbc34f2e147
2024-01-09 09:59:16 +01:00
Adam Wight 1434dc5ca6 Switch to a 1-based "count"
The previous patch deprecated the last conditional depending on magic
meanings of 0 and -1, so now we're free to let "count" take on a more
natural meaning: the number of times a footnote mark appears in
article text.

Includes a small hack to avoid changing parser output, by
artificially decrementing the count by one during rendering.  The
hack can be removed and test output updated in a separate patch.

Bug: T353227
Change-Id: I6f76c50357b274ff97321533e52f435798048268
2024-01-08 11:45:36 +01:00
Adam Wight 86edddc8c2 Use semantic field to test ref type
Stop relying on the magic number distinction between "count" = 0 and -1,
by explicitly testing the "name" field instead.

Bug: T353227
Change-Id: I9dce16b01814e19f508d45b927de570049f0e0f5
2024-01-06 16:46:56 +01:00
Adam Wight a6cb979d88 Inline constant for "placeholder" key
Minor refactoring of an internal field, which can be treated like the
other columns.

Change-Id: I255578694c5ab9f2ad3cbe232217af3cea60669c
2024-01-05 11:22:30 +00:00
Adam Wight fd648aec98 Store group in ref items
Encapsulate all information about a ref inside of the internal
structure, rather than relying on the container to be organized by
group.

Bug: T353451
Change-Id: I4c91e8089638b7655bf120402a4a5fcbd1b35452
2024-01-05 11:22:12 +00:00
Adam Wight ca6414320f More explicit test fixtures: key and count
These fields get automatic values during normal operation, but we
should make this explicit in tests which meddle with internals.  This
seems to add some clarity, and helps prepare for encapsulation.

Bug: T353451
Change-Id: I8b012a270f16139671f77ea04645d627b2fba87d
2024-01-05 11:21:58 +00:00
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
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 ab20cb3cdf Merge "Rename appendText() to resolveFollow()" 2024-01-04 15:29:44 +00: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