The idea is to make the code less ambiguous and easier to predict.
We passed the same information around two times in a few places.
Change-Id: I39c7a2962bb70bbe40074986e63b1051d0766ea2
With this change the <ol> is added after the inner list of <li> was
parsed as wikitext. In other words, the outer <ol> is now raw HTML
and not sanitized any more. This is fine because it's generated via
code. It doesn't contain any user input.
You might ask how it is possible to parse "invalid" HTML that
contains a sequence of <li> without the outer <ol>. This is fine
because the wikitext parser doesn't care about the nesting structure
of HTML elements. This is done later by Remex (Tidy). But Remex is
never called here. What we care about here is that the wikitext
parser sanitizes the individual HTML elements and their attributes.
The <ol> doesn't need sanitization.
This will make it possible to use reserved data attributes for
T196828. A bazillion unit and parser tests prove that this change
doesn't have any unwanted side-effects.
Bug: T196828
Bug: T239572
Change-Id: I0a9d419f48cad5ddb7251c8fdd2cf9506649436b
I'm importing the current keys from the Popups extension without
renaming. After that's done keys can be renamed and we can take
care off the fallback solutions.
Bug: T363156
Change-Id: I788c16c5bddc0df7f00dbbc39625b9adaa5bf184
This patch adds 'mw-cite-backlink' to the linkback span for both
named and unnamed refs. This requires us to add a span wrapper
for the unnamed refs case.
Verified in local testing that this causes aria attributes to be
added to the linkback tags in Parsoid HTML.
This should likely fix other gadgets and code that rely on this
class name to do their work.
Strictly speaking, this is a breaking change since we add an
extra span wrapper for the unnamed ref backlinks which *could*
break anyone using a li > a[rel="mw:referencedBy"] selector.
But, given the specificity of the a[rel] selector, the "li >"
part is unnecessary and might not be used. So, if we wanted to
push our luck (and break process), we could get this in.
Alternatively, we could:
- do this in the the read views OutputTransformPipeline.
- do a real major version bump -- we would be exercising that
functionality and have to fix and implement any missing pieces
that may have broke as part of the RESTBase sunsetting.
- not add the span wrapper and fix gadgets to explicitly look for
both named and unnamed refs with their selectors.
Bug: T328695
Change-Id: Icbd325ebd12cb42186c5b5220dc016835eb18b64
Page property is removed immediately since $wgCiteBookReferencing has
never been enabled in production.
Bug: T239989
Change-Id: I6252fcf1485994244dca40470cc5955e8d4f6917
This adds the 'reference-text' class where Parsoid added
'mw-reference-text'.
If we don't care about the "mw-" prefix, since there are very
few wiki and code references to 'mw-reference-text', it might
seem like we could update all those references and rip out
'mw-reference-text' from Parsoid output.
But, Parsoid HTML is also exposed via the REST API which means
there are likely many users out there analyzing Parsoid HTML.
https://github.com/search?q=%22mw-reference-text%22+NOT+language%3AHTML&type=code
says there are 512 references to this string - so looks like
we are probably going to rely on a major HTML version bump in
Parsoid in the future and then rip out all the duplicate
classes (mw-ref, mw-references, mw-reference-text OR
reference, references, reference-text).
Bug: T328695
Change-Id: I04b18ac75863a0e3e61bdd47b34508e5547dc872
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
I50557e0 changes the ExtensionTagHandler::lintHandler interface so it
can't return a node any more but must return true instead. True
indicates linting has been handled.
Depends-On: I441699e7fe9827a5e06e4638ce88c685deb9b856
Change-Id: I8fe4edc41a840c72cb539bf6f931d45ac777f8a0
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
* Improve PHPDoc documentation.
* Add some missing language-level type declarations.
* Remove unused code.
* Avoid count() when the code doesn't care about the actual count.
* Use the short ??= operator where possible.
Change-Id: I79de49b65d32661b7efa67ecc350276968943e11
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
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
This was added in 2006 via commit eb3a3f78, see
https://phabricator.wikimedia.org/rECITeb3a3f78
Hard to tell what happened back then. It's obviously not needed any
more, as proven by the tests. I mean, even if there would be an
extra newline character, it would be irrelevant at the end of an
<ol>…</ol>.
Change-Id: I5715cd9f31ac7ef86c1ea227642336ae71684291
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 commit also moves certain parser tests involving <ref> from
the Parsoid repo to citeParserTests.txt in this repo.
Bug: T354215
Change-Id: Ie5b211d2af01a56684473723c68a9ab2775542e3
The namespace change avoids a conflict with the existing Parsoid
implementation in Wikimedia\Parsoid\Ext\Cite and matches the current
Cite codebase better. We also need to add some phan stubs to allow
Cite to use Parsoid's generic DOM implementation classes, and some
type assertions to satisfy phan.
Bug: T354215
Change-Id: Ic904601b29555c9485a804f131061f207970ddd4
Parsoid's phpcs configuration is slightly different from the one in
this repository; this commit just keeps CI happy with the imported
code.
Change-Id: I9ce2993e8a9416f331b5157dfcfb01fb6e31baaf
Further commits will be necessary to complete the migration, but
this merge commit imports all of the existing history of the Cite
extension. It was generated using the following command on a checkout
of Parsoid:
git filter-repo --path src/Ext/Cite --path src/lib/ext/Cite \
--path lib/ext/Cite --path lib/ext/Cite.js --path lib/ext.Cite.js \
--path js/lib/ext.Cite.js --path modules/parser/ext.Cite.js \
--tag-rename '':'parsoid-' \
--path-rename src/Ext/Cite:src/Parsoid \
--path-rename src/lib/ext/Cite:src/Parsoid
And then, in the Cite repository:
git remote add parsoid ../path/to/parsoid/checkout
git merge parsoid/master --allow-unrelated-histories
Bug: T354215
Change-Id: I54edd9cf7951ca024c66fe357e8777eed85ab13b
Same as I294b59f in the Cite codebase.
An additional, necessary change is that we need to track all dir="…"
values in the ReferencesData object, even if we aren't going to use
the value from a <ref name="…" dir="…" /> reuse without content.
This is the same what's done in the ReferenceStack in the Cite
codebase.
Bug: T202593
Depends-On: I294b59f989f553932b40d08308906dd72d92d2cd
Change-Id: Ida38ae6a41e8550089cf7a37a549080d17943521
This test was obscured by testing for a field on the parent, but that
would exist if and only if the parent also existed. Clarify the
guard condition and introduce a named local variable for the parent.
Change-Id: I03079f45cf5ba00d54642c89ac4232a944b2f353
Such a message shouldn't exist, and doesn't:
https://global-search.toolforge.org/?q=.®ex=1&namespaces=8&title=Cite+link+label+group-
Additional notes:
* Rename the method to make it more obvious that it's not a cheap
getter, but doing something slightly more expensive.
* Use more appropriate array_key_exists to check if a cache entry
already exists.
* Also add a bit more documentation.
Bug: T297430
Bug: T353227
Change-Id: Ia5827bbf6fd700b87a749aac17320796428f0688
