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
The current tracking is wrong for several reasons. Mainly because
of a race condition if the Popups extension fininshed loading
before the Cite tracking script is executed. But further more
wgPopupsReferencePreviews was not a good choice to see if the user
sees previews or not.
The logging now uses the monoschema and only checks for enabled
previews when the click events are fired. The chances that Popups
finished initilizing then are much higher then. We still can see
if the init is not finished and the variable not set though.
Also we won't track the overall pageviews in here but use the
generic pageview_hourly from the data lake instead.
Bug: T353798
Depends-On: I1c434f0098ae23bd62256686a658e3d5ef7f70b9
Change-Id: I7a9524274efb58286f520c6148d5463bb0a78dbf
Steps to implement:
Copy over and adapt setup files, to install Cypress in the Cite code base.
Port tests/selenium/specs/backlinks.js and supporting file cite.page.js to run under the Cypress environment, in a second patchset.
Run the new suite in CI, replacing the previous selenium integration.
Delete the selenium test suite.
Bug: T353436
Change-Id: Ie76371e18d8612daa7c7be741432c6f3e0b783b5
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
Some interesting stuff is happening, seems to have revealed bugs:
* Rolled-back warnings are still present on the ref
* Subref reuse numbering starts at 0 instead of 1, and formatting is cringe.
But subref rollback does seem to work!
Change-Id: If6321b34d27370553ba85e63dd1e2ae6a3b7c099
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
This encapsulation gives us field name, type validation and code
documentation.
This patch only affects ReferenceStack and continues to return
approximately the same array outputs to callers. Some additional
information is included and the placeholder column has a new name.
Bug: T353451
Change-Id: I405fe7ac241f6991fd4c526bfbb58fbc34f2e147
The placeholder field will only be set if the ref exists, so we can
put these in a more logical order.
Change-Id: I2ddfb501fcc3aca936bb45c0d40e4f68c5d2b192
The previous patch deprecated the last conditional depending on magic
meanings of 0 and -1, so now we're free to let "count" take on a more
natural meaning: the number of times a footnote mark appears in
article text.
Includes a small hack to avoid changing parser output, by
artificially decrementing the count by one during rendering. The
hack can be removed and test output updated in a separate patch.
Bug: T353227
Change-Id: I6f76c50357b274ff97321533e52f435798048268
Stop relying on the magic number distinction between "count" = 0 and -1,
by explicitly testing the "name" field instead.
Bug: T353227
Change-Id: I9dce16b01814e19f508d45b927de570049f0e0f5
These can be hard to read so this patch introduces named, temporary
variables.
PHP reference assignment is helpful here, and has the nice property
of responding correctly to `isset` as if it were called on the
referenced variable. However, we're prevented from using this trick
in more places in the code because of an unfortunate side-effect that
PHP will store `null` under the referenced array key. In some cases
(the ones here), this is harmless because we always test using
`isset` and null behaves the same as an unset value. In other cases
such as arrays that are iterated over, the spurious key and null
value would be more of a nuisance.
Bug: T353227
Change-Id: Ie43592a2f10677ba19842e92fa29eb4bf3be240c
Encapsulate all information about a ref inside of the internal
structure, rather than relying on the container to be organized by
group.
Bug: T353451
Change-Id: I4c91e8089638b7655bf120402a4a5fcbd1b35452
These fields get automatic values during normal operation, but we
should make this explicit in tests which meddle with internals. This
seems to add some clarity, and helps prepare for encapsulation.
Bug: T353451
Change-Id: I8b012a270f16139671f77ea04645d627b2fba87d
In this case, there was never a ref with this name in the article so
no backlinks should be rendered.
TODO:
* test case with empty parent backlink and LDR parent
Bug: T353451
Change-Id: I8a7abd05a48ce83da3beb92b15e894d53252bd33
This is another improvement after I7390b68. Status objects are made
to keep track of multiple errors. The only difference is: The merge
method skips duplicates when the message and all parameters are
identical. This causes a minor user-facing change. One of the
shortest possible examples is:
<references>
<ref />
<ref />
</references>
This showed two identical, indistinguishable error messages before,
but will only show one now. We argue this is fine. The duplicates
are confusing and of (almost) no value to the user. In case the
information is relevant the correct solution is to make the error
messages distinguishable, or introduce a message like "multiple
<ref> tags defined in <references> have the same error". This is
something for a later patch, if needed.
Bug: T353266
Change-Id: I444105462ed24d5ba37b057622b4dc847b40f8d8
Testing internal methods is brittle. This code path is already
covered by parser test "Valid follow="…" after it's parent"
Bug: T353451
Change-Id: I3b7a4b9962de1f25a7b57f82d80813219d633594
