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
This is based on the popular 'count' parameter from Template:Reflist on
English Wikipedia, which has also been adopted by many other wikis.
That template's 'count' parameter allows maximum flexibility on a per-
page basis. This was important because the template can't know how many
references the list will contain. Users typically manually add (and
later, increment) the 'count' parameter when the list exceeds a certain
threshold.
The template currently sets an exact column count (via the CSS3
property `column-count`).
This patch improves on that by instead using the closely related CSS3
`column-width` property. This automatically derives the column count
based on the available space in the browser window. It will thus create
two or three columns on a typical desktop screen, and two or no columns
on a mobile device.
The specified width is the minimum width of a column. This ensures that
the list is not split when rendered on a narrow screen or mobile device.
It also hooks into the raw list before parsing and adds the class only
when the list will contain more than a certain number of items. This
prevents very short lists from being split into multiple columns.
Templates like Template:Reflist on English Wikipedia currently are not
able to set inline styles on the list element directly, which is why
they set it on a `<div>` wrapping the `<references />` output. Because
of this, the feature of the Cite extension must not be enabled at the
same time, as that would result in both the template's wrapper and the
references list being split. The end result would involve sitations with
three columns split in four sub-columns, creating a complicated mess of
nine intermixed columns.
To provide a smooth migration for wikis, this feature can be disabled by
default using `$wgCiteResponsiveReferences = false`. Each individual
template createing reference list can then be migrated, by removing the
wrapper column styles and instead settting the new "responsive"
attribute, like so: `<references responsive />`.
Once any conflicting templates have been migrated, the default for the
wiki can be swapped by setting `$wgCiteResponsiveReferences = true`.
If wikis wish for some templates to keep their custom column splitting
behaviour, templates can also opt-out by setting `responsive="0"`, which
will make sure that it will keep behaving the current way even after the
feature becomes enabled by default for the wiki.
In summary, when disabled by default, pages can opt into this system
with `<references responsive />`. When enabled by default, pages can opt
out of the system with `<references responsive=0 />`.
* Deprecate cite_references_prefix/cite_references_suffix.
This message is rarely used and opens up compatibility hazards.
It was already removed by Parsoid, but the PHP implementation
still had it. It's typically used to add inline styles to the
wrapper which is more appropiately done in Common.css (or
obsoleted as part of the skin or Cite extenion itself nowadays
depending on what style in question).
It was also a HTML-style message with separated open and close
segments, which is an anti-pattern in itself.
* Declare module target explicitly and include mobile. The absence of
this stylesheet caused subtle BiDi/RTL bugs on mobile.
Bug: T33597
Change-Id: Ia535f9b722e825e71e792b36356febc3bd444387
