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
This change does have two consequences:
1. A few more whitespace characters act as separators. This should not
have any consequence in real life situations, and is mainly done to
make the code easier to read and less surprising.
2. Sequences of two or more whitespace characters previously resulted
in partly *empty* results. This was a potential source of errors. The
additional + fixes this.
Change-Id: Ib58326109c740dd0cbd05d8fddb4af2145f232fe
Core sanitizes link targets and removes double spaces and underscores.
But the corresponding id="…" attributes are not sanitized the same
way. This results in broken links. This patch is not perfect (two
references with name="a_b" and name="a__b" will conflict), but the
best solution I can think of at the moment.
Bug: T184912
Change-Id: I9dbc916ad99269517d84c8ffb8581628d44a9f4e
The separate "ext.cite.a11y" module is kept for (temporary)
compatibility with cached HTML, and should be removed in about
a month.
Browser tests will be added in a separate patch.
Bug: T205270
Change-Id: I26fe41c328157233cc5b06d38d2ba0f7b036a853
The iodea is to make the code simpler and easier to read. If no
code uses this feature, all it does is making the code unnecessary
complex.
Change-Id: I22747712a691443a29b57831d3a6926275ad986b
When used as {{#tag:ref|...}}, references can contain strip markers,
and different strip markers can hide the same text, so unstrip before
comparing to avoid false warnings.
Bug: T205803
Change-Id: I059fd853d1eea07aa06cc85f80e463dd97fd171a
Because of how arrays are handled, phan-taint-check thought all
return values from refArg() were escaped, where really only $dir
was. We also split the error method into the parse and noparse
case as separate functions so that phan can better analyse these calls.
In linkRef() we suppress the double escaping as the escaping used
is appropriate for inserting into wikitext.
Bug: T195009
Change-Id: I3e04c8cceae727e5470d4ae4fdb2404639f9bf33
@static is intended for use only when the language does
not support the concept of static methods natively
Change-Id: I9a0bf7db493d5667b22508e65a34034cefdbcbfa
The Cite extension already had a recursion guard around the parsing of
`<references/>`, to prevent another `<ref>` containing `<references/>`
from producing a weirdly nested references list.
When an explicit `<references/>` tag is not included in the page, or
`<ref>` tags exist after the last explicit `<references/>`, the extension
automatically adds a reference list at the end of the page, to make the
references still displayed.
This automatic references list creation was bypassing the recursion
guard, causing the weirdly nested output *and* a PHP Notice from
`mRefs[$group]` becoming undefined. This commit sets the recursion guard
state during that automatic references list creation to prevent this.
Bug: T182929
Change-Id: I87737dcf39a4fc15e119a1090a9c34d6b9633c21
The motivation for this patch is to make the code less complex, better
readable, and less brittle.
Example:
public function onExampleHook( Parser &$parser, array &$result ) {
/* This is the hook handler */
}
In this example the $result array is meant to be manipulated by the
hook handler. Changes should become visible to the caller. Since PHP
passes arrays by value, the & is needed to make this possible.
But the & is misplaced in pretty much all cases where the parameter is
an object. The only reason we still see these & in many hook handlers
is historical: PHP 4 passed objects by value, which potentially caused
expensive cloning. This was prevented with the &.
Since PHP 5 objects are passed by reference. However, this did not
made the & entirely meaningless. Keeping the & means callees are
allowed to replace passed objects with new ones. The & makes it look
like a function might intentionally replace a passed object, which is
unintended and actually scary in cases like the Parser. Luckily all
Hooks::run I have seen so far ignore unintended out-values. So even if
a hook handler tries to do something bad like replacing the Parser
with a different one, this would not have an effect.
Removing the & does not remove the possibility to manipulate the
object. Changes done to public properties are still visible to the
caller.
Unfortunately these & cannot be removed from the callers as long as
there is a single callee expecting a reference. This patch reduces the
number of such problematic callees.
Change-Id: Ib3a9da257b50326d569ab1973b523c952963c16b
This is the default for many years now. Returning true does nothing. It's
identical to returning nothing (null). The only meaningful value a hook
handler can return is false, and even this is meaningful only for very
few hooks.
TL;DR: A "return true" in a hook handler is always meaningless, dead code.
I'm interested in this because we (WMDE) might start working on this
extension soon and I want the code to be small and easy to maintain.
Change-Id: If4f32a55cdc38a3cc8af286d1cca7c0089bbfc43
Adding option for dir attribute in ref tags. The value must be a valid
direction ('ltr' or 'rtl', case insensitive) or the direction will be
stripped out.
The directionality of the li element is set using a css class accordingly.
Bug: T15673
Change-Id: Iff480bc8cc4f81403b310e8efecd43e29d1d4449
Since Cite requires 1.25+ now, the checks for PPFrame::setVolatile(),
which was introduced in 1.24, can be removed.
Change-Id: I91df2e91b2f7a21b2b1147aa6af194980527f86b
This was briefly enabled in WMF production in 2009 and found not to work.
As far as I know, it's been disabled since then. Retaining it requires
maintaining the complex "half-parsed serialization" feature in the core
parser, which I'm deprecating in I838d7ac7f9a218. The core feature was
added solely to support this Cite caching experiment and is not used for
anything else.
Change-Id: I446e0c46913a390dbdf7b49b84040bf47ed6c2f9
Don't use the \Database alias, use the namespaced version when calling
Database::getCacheSetOptions.
And document why the remaining issue is suppressed.
Change-Id: I80a102f2e82efedcfa999d8e714bfe049263ffeb
Line 334 of Cite/includes/Cite.php contains two preg_match () calls. The subject lines for them are produced by Cite::refArgs () and are set to null or false when no name or follow attribute is provided in the <ref></ref> tag.
However, preg_match () is supposed to accept only strings as its subject, and the nowhere in the documentation it is said that it is nullable.
At least, in HHVM 3.12 this causes an exception.
The enclosed patch adds simple checks making sure that preg_match () is not called when $key or $follow are null or false.
Change-Id: I3e00d31d6bf216271ace7e851d88c68c4fd5ed00
When using HTML5 ids, we need to take greater care to properly escape the
id (or derived strings) before passing them back through
Parser::recursiveTagParse().
Bug: T176170
Change-Id: I89a4f8ba24b867f2d5ccdc2bf9a4312ab9b385a9
