We are *so* close to 90%.
This patch should raise the coverage for the CiteDataModule to 100%.
I'm also adding a pure unit test for the clone() behavior. Note the
later is already covered by the CiteDbTest.
Question: Do we want the CiteDbTest to @cover anything?
Change-Id: I40763d01e18991f509bc30b6655aa57b23412fd9
Fixes a bug introduced in Icf61c9a27fd, which would cause a parser
cache split any time the Cite extension was initialized. The
`setLanguage` interface is regrettable, but I'm hoping it will only
be around temporarily.
Converts an integration test into a unit test and completes coverage.
Bug: T239988
Change-Id: I4b1f8909700845c9fa0cbc1a3de50ee7d42f69a5
Tickle very particular edge case in which a recursive parse corrupts
the $parser->extCite object.
Bug: T240248
Change-Id: I70d100e88fa72825194ed9c477b030bbf0b6b486
Because that is what it does. Note our method is different from the one
in the Language class. We only accept strings.
Change-Id: I39107e837cc29f2d7c8867c1e602aa643f9e1a57
This is what happens:
* The issue happens only on pages with two <ref> tags than share the
same name and group, but have conflicting text.
* This triggers a code path that renders an error message and calls
Message::plain() as well as Parser::addTrackingCategory(), which calls
Message::text().
* The Message class is asking for a new, fresh parser. This means the
parser is cloned and it's state cleared, while keeping stuff like
parser hooks.
* Cloning the parser triggers the ParserCloned hook.
* The hook handler clones the Cite instance stored in Parser::$extCite.
* PHP doesn't do deep cloning. Object properties are not cloned.
* Since I091a0b7 the internal state of the Cite class is extracted to
another class.
* This means the state is not cloned any more since I091a0b7.
* Now two Cite instances share the same state.
* At the end of the hook handler, the state is cleared, which also
clears the state of the original instance.
We will most probably solve this on master by getting rid of cloning
Cite. We propose this additional hotfix for the branch.
Bug: T240248
Change-Id: Ic5a438e04d003a637ae08aae936d9977cc90d5d3
This class renders a <references> tag and everything inside. The
previous name sounds like it is responsible for rendering the contents
of a <ref>…</ref> tag. I mean, the class contains a method that does
exactly this. But this method is private.
Change-Id: I1cd06c9a11e0a74104f2874a34efa3e0843a0f70
This adds a test for numbers like "1.2.0" that appear when an extended
reference (e.g. "1.2.") is reused multiple times.
The first separator is from the extended reference. We decided to never
localize it. However, the second seperator is from reusing a reference.
This was always localized. We believe this is a bug, but haven't fixed
it yet.
The test is documenting the status quo "1.2,0" with a comma. This kind
of makes sense, one could argue, because the "1.2" appears like this up
in the text, but the ",0" is a different indicator for a reuse, which
*never* occurs in the text.
Change-Id: Ie3d26bcadd8929b906bfbcac4806af2150d61f2a
The flag wgCiteBookReferencing is disabled by default. It looks like
we forgot to add these lines because:
* We enabled the flag on our personal dev environments.
* It's enabled on CI as well. That's why no test failed (previously).
Change-Id: Ifd967adf58cd0ddf7b054d0321412f23fa522ad7
Before, this regular expression was looking for incomplete wikitext
like this:
<ref>unclosed
<ref>closed</ref>
With this change, wikitext like this will trigger the same error:
<ref>unclosed
<references />
incomplete</ref>
This should be much, much more rare. But I feel it's reasonable to mark
this as an error, instead of just rendering the broken inner tag in
plain text.
This patch also replaces `.*?>` with `[^>]*+>`. Both do the exact same.
Instead of doing an "ungreedy search for the first possible closing
bracket", which might cause backtracking, the new syntax consumes all
non-brackets before expecting one. This is guaranteed to never backtrack
(guaranteed by the extra +), and potentially faster because of this.
Change-Id: Ic76a52cd111b28e4522f095ce3984e3583f602c1
The concept "key" already exists in the structure handled by this
function, so to have a $key which means something else was distracting.
Change-Id: I91a76edbb42a1ab6514bc706b75ab89f78539fa5
This partly reverts Ied2e3f5. I haven't properly tested this before.
Rendering a bad extends (that extends a <ref> that's already extended)
not indented messes the order up and rips other extended <ref>s out of
context.
For now it might be better to stick to the previous, "magic" behavior:
Such an extends behaves like it is extending the *parent*, and is
ordered and indented as such. This is still not correct, but I feel
this is much better than rendering such a bad extends on the top level.
This patch also makes the code fail much earlier for a nested extends,
if this decision can be made already. In this case the error message is
rendered in the middle of the text (as other errors also are), not in
the <references> section.
Change-Id: I33c6a763cd6c11df09d10dfab73f955ed15e9d36
This partly reverts Id7a4036e64920acdeccb4dfcf6bef31d0e5657ab.
The message "cite_section_preview_references" says "Preview of references".
This line is not meant to be part of the content, but an interface message.
It should use the users (interface) language, not the content language.
Change-Id: I1b1b5106266606eb0dfaa31f4abd3cee9ba92e8c
These edge cases are handled correctly already, I just forgot to
remove the TODOs when updating test content.
Note that there's only one TODO left, and it's to forbid a feature which
actually works!
Change-Id: I0d3a1f55f0ce943b0d034dda40e3779fbf241fe4
Same as Ib6e9de6.
We must reset the build-in "list-item" counter to make this code behave
sane in Firefox. It looks like this is even described in the CSS spec
and it is not Firefox having a bug, but Chrome being "clever" and not
following the spec.
Bug: T229307
Change-Id: I955786e2b68d087c819a962ded3c571946c61f78
All mock screenshots on
https://de.wikipedia.org/wiki/Wikipedia:Technische_W%C3%BCnsche/Topw%C3%BCnsche/Erweiterung_der_Einzelnachweise
show this extra dot.
Note the numbering scheme "1." and "1.1" (without the dot) was not a
mistake. It's not uncommon to only have the extra dot on the first
level.
However, I feel the scheme "1." and "1.1." might be a little more
consistent and less surprising.
Change-Id: I0dc467755926f82d88a48fb7594af0bde8bbee21
This simplifies as well as fixes a series of issues with this regular
expression:
* Before, the wikitext `<REF><REF>` would not trigger the error, but
`<ref><ref>` would. Parser tags are case-insensitive, but the error
check was not.
* Before, the wikitext `<ref><ref name="<">` would not trigger the error.
That's a valid name. The error check should not stop just because it
found a `<`.
* Both the old and the new code do *not* fail with the wikitext
`<ref><ref</ref>` where the inner `<ref` does not have a closing `>`. I
was thinking about changing this, but figured it might be used as a
feature.
* The old code was not able to properly understand HTML comments,
<nowiki> tags and such that contain a line break. That caused
inconsistent and confusing error reporting in some cases, but not in
others. This change *reduces* the amount of errors this code produces.
* The old code was looking for "SGML tags" with names that could be
anything, not just alphanumeric characters. This allowed for strange
edge-cases like `<ref><>><ref></>></ref>` that have not been reported,
but should be. This change *increases* the amount of errors. However,
relevant edge-cases should be extremely rare.
Note the ++ avoids backtracking, speeding up the regex.
Change-Id: I0c61a245f4f743871b4cad886ce239650af2b37c
