Two motivations:
1. I want the two deeper nested functions guardedRef() and
guardedReferences() to have less side effects.
2. In guardedReferences() guardedRef() is called. Both set the
property. That's redundant. The new code avoids this.
Change-Id: I48146f8b6d91122a904be0a552ffe3b03bc0481f
Main motivation is to make the code easier to test, and easier to
extract to smaller services.
Does this make sense? I'm not sure any more. One can argue that
everything Cite does happens in the context of a specific Parser. Why
shouldn't the code have access to this Parser?
Change-Id: I9d0cb44d96ec70a56af57f86aeb1f264f52c8bc4
Note it doesn't make a difference if this is behind the feature flag or
not. It should always be forbidden, and in fact is: Either the follows
attribute is unknown, or the combination is forbidden.
Bug: T236256
Change-Id: Iebbb2d1d5bab183ab0590b8a7a7f6e79d319b72c
Any time the book referencing attribute is used in a page,
permanently tag that page with the `ref-extends` property, so
that it can be watched and cleaned up if necessary.
Bug: T237531
Change-Id: Ice5d9d8f7a305702cdc7c2a55d4147c4f79b5881
Incremental patch which extracts the refines attribute from the tag.
Doing this now to allow the calling function to have responsibility
for doing something with the attribute value.
Bug: T237531
Change-Id: I59bb409bedd8e6ed06268e705e02e8ffb45b1f0e
I was particularly suprised by the conditions that checked if
`$parser->extCite !== $this`. This can never happen. Maybe it was
possible in a very old version of this code, but it is not any more.
Change-Id: I049ff4109a747eb9dbf325c24cf20f65753827dd
As of now, this patch does not touch the existing code. However, the
goal is to remove a lot of the related code from the Cite class. This
will be done in later patches. This here is a separate patch to make
reviewing the later patches much easier.
The existing parser tests should be proof enough this chain of patches
is not changing any behavior.
Change-Id: I27ae972f81071bb4036bd452560768fae409417b
Renaming to $text since this is also already used in the array structure
to store the references and seems more intuitive than $str
Change-Id: I4dbe5d10ddc355b4587d195b50cf078ac01fac55
The text could be null if we're stacking a re-used reference there.
Also content is a more precise word for what was forwarded here.
Change-Id: Ic78fb4744314c40360a61c21e92462b6eb2ae1ab
I tried to avoid unrelated refactorings. The only thing I ended doing
is turning a huge if-else around into a guard-clause.
Main motivation here is:
* Make the huge, almost 2000 line Cite class smaller.
* Turn public code into private implementation details.
Bug: T236260
Change-Id: Ifca28040ae60d021a31aaee65417c7584627a975
All other hook handlers are in the dedicated CiteHooks class.
Main motivation here is to make the huge Cite class smaller,
especially by removing static code that does not rely on anything
else the class does.
Bug: T236260
Change-Id: If0b3f6c989e44283428cda4b2c4d8d5303385d22
Relevant edge-cases:
* ctype_digit() only works on strings. Anything else, including
integers, will make it return false.
* The empty string will return false.
Both is identical to what the code did before.
Motivation for this change is to streamline the code, and make it
smaller and faster to read.
Bug: T236260
Change-Id: I2d209347d16f2bde14b345c3f88ec64b081283cb
Allows the "refines" attribute when the feature flag is set, but doesn't
render. This is part of our rollback strategy, so that we aren't left
with invalid wikitext in case of undeployment.
Bug: T236257
Change-Id: I936be0e62dccb46caeb84162d2c5166956fd9916
* I used https://codesearch.wmflabs.org to make sure the private
constants are indeed not used anywhere.
* The added type hints are safe, as far as I can tell. There is no way
one of these parameters can contain anything else. Otherwise the code
would fail already.
Change-Id: Iaa7615e9864805760fa652700b58b69680b4f17e
Collect EventLogging metrics for footnote and reference link
interactions, so that we can compare behavior with and without
Reference Previews enabled.
This tracking will be reverted once analysis is complete.
A mostly arbitrary sample rate of 1/1000 is hardcoded here. This is
loosely based on the latest tuning of Popups sampling at 1/100,
divided by a conservative factor of 10 to ensure headroom.
The sample is skewed by skipping clients without sendBeacon support,
but we're avoiding the mw.track synchronous fallback, which injects an
image tag and introduces lag any time the user clicks external links
in the references.
Bug: T231529
Change-Id: Iad32b64114f88675eecbb01712418c968e3cf661
Core now uses the extension name to check if the Cite extension is
loaded. Therefore the class name could be changed in that regard.
Change-Id: Ibdc0725045f7a0b0afcbf6cb94ccdab9509ad672
Depend-On: I35e5aa9955141b575de68a5be2c0d5b87585eb77
We can resolve this bug by either replacing the bogus "return false"
with the intended "return [ false, … ]". Or rely on the code a few
lines below that also bails out with a "return [ false, … ]" when to
many parameters ($cnt is not 0 then) are present. The tests prove both
solutions are equally valid.
Bug: T211576
Change-Id: Iadd55c134dede7042cfd152c69bc8f27b59d8912
The biggest issue with this code was that it was tracking the exact
same state in two ways: Processed array elements got removed from the
$argv array, *and* the $cnt was decremented the same time. This is not
necessary and a potential source of confusion and errors.
I carefully transformed the code. I'm sure it still does exactly what
it did before. The tests should prove this.
Change-Id: I642d38e7944aa3e2239179fa58e1e231b4698263
The two message are not needed in case there is only one element.
Since fetching messages is a little expensive, I feel it's worth
rearranging this code.
Or not, because it seems this method is never called with one
element only.
Change-Id: Ie915278b41f053afe0d14a29d2aec54c98e5185e
