Also simplify the @covers annotations while we are here. The class
is really simple. There isn't much that can be covered acidentally.
Change-Id: I105f4ea6d6beb119d1557a32b691e9eda1b8085c
Introduce a static method so we don't need to copy paste code.
Note that the static method still largely duplicates what the method
.buildIndex() will later do. Both loops iterate the reference groups
and the references in each group. The main difference is that the
"is empty" check stops extremely early the moment it finds any
non-empty group.
That's also why I'm convinced it's not worth caching the result.
I benchmarked it and it's nanoseconds. But there are more reasons:
The non-static .isIndexEmpty() method is currently only used when
Citoid is active the same time. Which means the cached result was
entirely unused on installations without Citoid.
Bug: T356871
Change-Id: Id5c4295086bc977ef52ad141be9962d2eecb1bcc
Cite tests are blocking Citoid patches, by failing in that
environment.
TODO: To be reverted once the incompatibility is resolved.
Bug: T364382
Change-Id: Ie8eeac79966ccb46389e4f9aaa4620e9bd75cc00
The two are different:
* CiteReferencePreviews as specified in extension.json is a feature
flag that allows us to disable the feature entirely. It could be
named "CiteReferencePreviewsFeature" or "CiteEnableReferencePreviews",
but renaming a feature flag that's already in use is hard.
* The client-side flag tells the JavaScript code "you know what, it
was kind of a mistake you got loaded, please stop". This is because
we can not make all decisions before we register the ResourceLoader
module, e.g. if the user has certain gadgets enabled.
Adding the word "Active" is not a huge improvement, but at least
makes the two different now. Suggestions welcome.
Bug: T362771
Change-Id: I0f6a911df8772616ac50c1301f402f77dbe32089
PHP classes and test are somewhat copies from the Popups codebase.
Some refactoring was applied. More could be done. Not to sure if
this should happen more in follow ups though.
Could also reduce the complexity of checks on the JS side. Most of
these things can only change on page load. The only dynamic part
left is the anon user setting managed by the Popups extension.
Note, that I needed to add a new PHP config for here although the
other still exists and is needed in the Popups extension. This
will change, when the user settings code also moves.
I guess it's okay for now though. Both settings default to true
and are not overridden in the config repos.
Also needed to add the Gadget extension as phan dependency.
Bug: T362771
Depends-On: Ia028c41f8aaa1c522dfc7c372e1ce51e40933a5e
Change-Id: Ie6e8bc706235724494036c7f0d873f5c996c46e6
This updates the easy variables which are set and get within a single
module, these changes should have no effect and don't need to be
coordinated across extensions.
Bug: T362332
Change-Id: Ibbe69c321e9e2b744ec88cebbdc3476d776f5956
The ::setPageProperty() method has some tricky corner cases where the
type of the value determines whether or not the page property will be
sorted. Since sort order for the BOOK_REF_PROPERTY is irrelevant,
use ::setUnsortedPageProperty() to communicate this clearly to the
reader.
Depends-On: Ia94c192c429d0482c58467bed787fd2e0aca052f
Followup-To: Ibfd84b52057baa8e249d321ec9df612efd6a29a6
Change-Id: I399f4895ec8720ff2927c5cd5a09c7af4664ee46
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
The search index is really only used in a single place, in the
buildSearchResults method at the very bottom of the class. I find it
more obvious to understand what's going on when the places where the
search index is populated and used are as close together as possible.
This again really only moves existing code around without actually
changing anything.
We can also drop the extra "built" property and use a special null
value instead. This is possible because we know the only consumer of
the this.index property and can guarantee it can't get confused by
the null.
Bug: T356871
Change-Id: Iaddb3b16b3aa776f89fca2bf0350cce9b6bb1a23
This turns two methods with side-effects into more pure functions
with more obvious input and output. buildSearchIndex rebuilds the
internal search index from the internalList. buildSearchResults
filters and creates the result widgets the user will see.
This patch really only moves existing code around but doesn't change
anything. Except that this.built is set before onQueryChange is
called, not after. This avoids potential endless loops in case
onQueryChange happens to trigger buildIndex again.
Bug: T356871
Change-Id: Ib80a2dcb85779d64bec53caf90c49879d0ea2258
There is currently no test coverage for recursively parsing the
contents of a <ref>…</ref> together with an incomplete follow="…".
This is critical because that's an entirely separate, special code
path (the one that creates a <p> instead of an <li>). Without this
test we could return unparsed wikitext and never notice.
I discovered this while playing with I0b0e358.
Bug: T245549
Change-Id: Ie65c6bf6bf75db26e0fff733c93cfa28ee7bd228
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
The ext.cite.referencePreviews module will transparently replace the
ext.popups.referencePreviews module after this patch. Configuration
stays in Popups for now, we can migrate it in later work.
CSS classes may be renamed in the future but this will be handled
separately since it could be a breaking change for on-wiki
customizations.
A lot of fancy footwork happens in this patch to emulate a soft
dependency on Popups. This mechanism doesn't exist explicitly in
either ResourceLoader or QUnit, so lots of workarounds are used, to
conditionally load the module and to dynamically skip dependent tests.
renderer.test.js is fully skipped for now, but can be wired up in
later work.
Bug: T355194
Change-Id: I0dc47abb59a40d4e41e7dda0eb7b415a2e1ae508
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
The first two files have been added to the root modules/ directory
via I487095d in 2015. No problem.
Many, many more files have been added via I000b453 in 2022. It's
really hard to tell what is what since then.
I'm not absolutely sure what the naming convention for this folder
should be. Could as well be "localized-styles/" or just "Parsoid/".
Bug: T156350
Change-Id: Ibcf8c7a6db5400ed8a9811244a070e03ff372a39
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
The message was part of the original patch that introduced the group
feature in 2009, see https://phabricator.wikimedia.org/rECIT75004e33.
Notice how there was never a test scenario for this message. A test
was added in 2020 via I07738cc.
The message appears only in a rare edge-case when a group is entirely
unused in the text, and only when the group is not empty. The shortest
possible example is:
<references group=g>
<ref group=g name=a>a</ref>
</references>
Just adding something unrelated like `<ref group=g>x</ref>` to the
text changes the error message. Now the group is "used". But this
notion is confusing to begin with. References can be part of a group,
and we can use references, but we can't use groups as if they are a
separate entity.
A better error message already exists.
Notice how this special error message doesn't appear anywhere in the
Parsoid code path. That was already using the other, more generic
error message.
Bug: T269531
Change-Id: I63f663d76e45e6c3d664f145d9a564ee00ff53cd
This is about the error message that currently says:
»Cite error: <ref> tag with name "a" defined in <references> has
group attribute "" which does not appear in prior text.«
This is a special error message that appears only when a group name
does not appear anywhere in the text. In all other cases a simpler
error message is shown:
»Cite error: <ref> tag with name "a" defined in <references> is
not used in prior text.«
While the first error message is not wrong in the edge-case
described in T269531, it's very confusing for a multitude of
reasons. For example:
* There is no group attribute in the wikitext.
* Just adding something completely unrelated like `<ref>x</ref>` to
the text shows the other error message.
The reason for this behavior is that the assumed default is an empty
`group=""`. The error message changes the moment any other <ref> in
the same group appears in the text vs. when the group is entirely
unused.
We can probably remove this error message entirely, but should at
least not use it when there is no group.
Notice how the Parsoid code path was already using the other error
message.
Bug: T269531
Change-Id: Ifa2e97254f4cda72233a057d8760fb1116143552
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
… 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
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
* This is to worka round some confusing html2html failures in CI
for these tests (that are not reproducible locally for me).
Change-Id: I07725155ef5e04eb4346a90c34cbacbd70e88ea6
These tests pass today because Parsoid is providing an
alternative implementation of Cite, but that means this
test case isn't actually testing the code in this repo.
Bug: T354215
Change-Id: I42521026bab36035ae5eded7c05716234a5a29ea
General improvements
Ensure proper module loading in Cypress tests to prevent failures by waiting for specific modules to be loaded
Delete the selenium test suite.
Bug: T353436
Change-Id: Ieb71b122d3c6513f1a15d6574967e2bb9c21a393
