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
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
Same as Icfa8215 where we removed the …_suffix messages.
This patch is not blocked on anything according to CodeSearch:
https://codesearch.wmcloud.org/search/?q=cite_references%3F_link_prefix
According to GlobalSearch there are 2 usages we need to talk about:
https://global-search.toolforge.org/?q=.®ex=1&namespaces=8&title=Cite.references%3F.link.prefix.*
zh.wiktionary replaces "cite_ref-" with "_ref-", and "cite_note-"
with "_note-", i.e. they did nothing but remove the word "cite". This
happened in 2006, with no explanation.
ka.wikibooks and ka.wikiquote replace "cite_note-" with "_შენიშვნა-",
which translates back to "_note-". One user did this in 2007,
16 seconds apart.
It appears like both are attempts to localize what can be localized,
no matter if it's really necessary or not.
https://zh.wiktionary.org/wiki/Special:Contributions/Shibo77?offset=20060510https://ka.wikiquote.org/wiki/Special:Contributions/Trulala?offset=20070219
Note how one user experimented with an "a" in some of the edits to
see what effect the change might have, to imediatelly revert it.
The modifications don't really have an effect on anything, except on
the anchors in the resulting <a href="#_ref-5"> and <sup id="_ref-5">
HTML. It might also be briefly visible in the browser's address bar
when such a link is clicked. We can only assume the two users did this
to make the URL appear shorter (?). A discussion apparently never
happened. Bot users are inactive.
Both pieces of HTML are generated in the Cite code. Removing the
messages will change all places the same time. All links will
continue to work. The only possible effect is that hard-coded
weblinks to an individual reference will link to the top of the
article instead. But:
a) This is extremely unlikely to happen. There is no reason to link
to a reference from outside of the article.
b) Such links are not guaranteed to work anyway as they can break
for a multitude of other reasons, e.g. the <ref> being renamed,
removed, or replaced.
c) Even if such a link breaks, it still links to the correct article.
There is also no on-wiki code on zh.wiktionary that would do anything
with the shortened prefix:
https://zh.wiktionary.org/w/index.php?search=insource%3A%2F_%28ref%7Cnote%29-%2F&title=Special%3A%E6%90%9C%E7%B4%A2&profile=advanced&fulltext=1&ns2=1&ns4=1&ns8=1&ns10=1&ns12=1&ns828=1&ns2300=1
I argue this is safe to remove, even without contacting the mentioned
communities first.
Bug: T321217
Change-Id: I160a119710dc35679dbdc2f39ddf453dbd5a5dfa
This fixes a minor issue introduced in I294b59f. Two identical
dir="…" with different capitalizations should not be reported as an
error.
Turns out the implementation in the Cite extension doesn't care
about this capitalization at all. That's why I suggest to do the
normalization as early as possible. This is slightly different in
the Parsoid implementation.
Bug: T202593
Change-Id: I96b4a281d6020d61d1f36ec027cf833bbb244f03
* Same as Ie64f4ab in the Cite codebase.
* Mark the changed tests as standalone since this Parsoid code isn't yet
released to vendor and integrated tests run with vendor.
Bug: T299280
Depends-On: Ie64f4ab4831966f66f812ea67cc244718f818afb
Change-Id: I0ea1bc3f57576d215ba4060a0e886e588ffda0b3
Internal ref key is always an int, but another string `key` is
created in the formatters. This patch makes the typing explicit. We
can distinguish between these two different values in a later patch.
Bug: T353451
Change-Id: Id5e40517705961f4d54622e91264430d9f62008d
Thanks to strict types and a recent MediaWiki CodeSniffer update a
lot of the PHPDoc comments in this codebase became redundant. Only
very few comments in this codebase contain additional information.
Such comments don't add any new information to what the code alone
already says. We started removing them in many other codebases
already.
In case someone wants to add more documentation to a method the
basic PHPDoc block can usually automatically be generated with a
button press in the IDE.
The only additional change in this patch is that I occasionally
add a missing `void` return type. This is necessary to be able to
remove the comment.
Change-Id: Id7d6d6a437175a9d017f564daf7ed16e76f09158
This is doing the same as before, in pretty much the same execution
order. The only difference is the syntax.
In JavaScript it's relevant to not do array initializations to early.
Otherwise different instances share the same array. But this doesn't
happen in PHP.
Change-Id: I56363ccadf29f2b806f765ab8f54a3c1863fc10f
I'm not sure how much this helps. But this merges two code paths
that are both about "we are in the middle of a <references> section
right now.
Nothing changes, as proven by the tests.
Bug: T353266
Change-Id: I446e224b81d35c47736a437d78527c0cc8636f77
This classifies as a "warning" because we still show everything,
just with an error message appended.
Disabling the Parsoid tests right away hopefully makes it easier to
do the same change in Parsoid.
Bug: T202593
Depends-On: If14acd1070617ca8c4d15be6b1759bd47ead4926
Change-Id: I294b59f989f553932b40d08308906dd72d92d2cd
By now I'm sure this really doesn't belong here. The code in the Cite
extension is doing this because it generates HTML by concatenating
plain strings. In such a context the necessary HTML entity encoding
(" and such) must be done manually. Here in the Parsoid context
this is not needed.
This is split from I7249bd0. See the discussion over there.
Change-Id: I5589e5c2147bfc9f205a0ff80d8bdd247ab49c63
* This partly replicates the fixes in I9435a2d and Ia01f2fd. More
to be done in later patches.
* Updated html/parsoid test output (which matches the change in the
html/php section).
Depends-On: I401656265253a429691cc76adc5db5b129cff2cc
Change-Id: I7249bd03a7942ff7725a20178a051300b777e3a8
This moves one more error situation into the stack class, together
with other error situations that are already there.
Bug: T353266
Change-Id: Icf169650f67f64e6d29d175c3b47cf558b8de3d4
Check out how this gets rid of so many "to do" as well as
"deprecated" comments.
Next qustion: The elements in the stack become more and more
complicated. It's probably worth converting them from arrays into
first-class objects. But this is for another patch.
Bug: T353266
Change-Id: If14acd1070617ca8c4d15be6b1759bd47ead4926
We are discussing this for a long time and finally renamed the tag
on Phabricator: https://phabricator.wikimedia.org/tag/cite-extends
This patch updates only places where it can't have any negative
consequences.
This is also a direct follow-up to Ic73f1b7 where this class was
created.
Bug: T353269
Change-Id: I644fe41d3386b9bf02b83366654301633efd535f
Same arguments as in Iafa2412. The one reason to use more detailled
per-method @covers annotations is to avoid "accidental coverage"
where code is marked as being covered by tests that don't assert
anything that would be meaningful for this code. This is especially a
problem with older, bigger classes with lots of side effects.
But all the new classes we introduced over the years are small, with
predictable, local effects.
That's also why we keep the more detailled @covers annotations for
the original Cite class.
Bug: T353227
Bug: T353269
Change-Id: I69850f4d740d8ad5a7c2368b9068dc91e47cc797
This is a concept that's only relevant when a sub-reference (formerly
known as BookReferencing) appears before the parent reference it
belongs to. Let the name reflect this.
Bug: T353227
Change-Id: Iabf259e72942ea70cb1cc1e0ca5a5d8cf15d7225
This patch only moves existing code around without changing any
behavior. What I basically did was merging the old "guardedReferences"
method into "references", and then splitting the resulting code in
other ways. Now we see a few other concepts emerging. But the idea
something would be "guarded" (how?) is gone.
The most critical detail in this patch are the new method names, and
how the code is split. The names should tell a story, and the methods
should do exactly what the name says. Suggestions?
Bug: T353266
Change-Id: I8b7921ce24487e9657e4193ea6a2e3e7d7b0b1c3
This removes almost 200 lines from the main class.
This patch intentionally doesn't make any changes to the code but
only moves it around. Further improvements are for later patches.
Bug: T353269
Change-Id: Ic73f1b7458b3f7b7b89806a88a1111161e3cf094
Allow other extensions to provide lists of page content
models for which they want to load the Cite toolbar button.
This will, for example, make it possible for ProofreadPage
to have the button on Page pages.
Bug: T348403
Change-Id: Id28cb0b6cb8a2b86a66b17232575afe513969c54
The main benefit is that the two lines that set and reset
$this->inReferencesGroup are now next to each other. More can be
done in later patches.
Bug: T353266
Change-Id: Ib3f40c40e0b1854f8e5a32af600f28931fffdb8c
I played around with a few options (see patchset 1) but ended
introducing new terminology:
* "Backlink" describes the ↑ button down in the list of <references>
that jumps back up into the article. The code was already using
"backlink" in some places.
* "Backlink target" is the id="…" attribute up there, visible as the
typical [1] in the article.
* I use "jump" to describe the idea that clicking the [1] jumps down
to the full reference.
* "Jump target" is the id="…" down there in the list of <references>.
* "Jump link" is the same id, but encoded to be used as the href="…"
attribute when clicking the [1].
I hope this makes sense. Suggestions welcome.
Another benefit is that "normalization" is really only normalization
now, not any URL and/or HTML encoding.
Bug: T298278
Change-Id: I5a64ac43aef895110b61df65b27f683b131886fb
This moves the actual parsing down to be done much later in the
process. This won't make any difference in production but makes it
easier to refactor the code further.
Note I tried to use a StatusValue object but couldn't because it
merges seemingly identical messages, while the plain array is fine
with containing duplicates. There is one parser test that covers
this. While we could change this it needs discussion and most
probably a PM decision.
Change-Id: I7390b688a33dace95753470a927bbe4de43ea03a
The "parser marker" placeholders are case-sensitive, e.g. for a tag
that's written like <rEf> the placeholder will also say …-rEf-…. This
was really just a mistake.
The error is as old as this code is. Added in commit 75004e33 in
2009.
Note we shouldn't use /i at the end because the marker itself should
not be case-insensitive. Only the tag name.
Instead of adding more (slow) test cases I update two that are
exactly about this part of Cite (nested tags) anyway.
Bug: T64335
Change-Id: I44c7a42a0da682a1082952fd1af817bf7d45378c
This error message really always meant nothing but "there is an
unknown parameter in your <ref> tag". It's unnecessarily confusing
only for historical reasons. See T299280#9384546 for a long
explanation.
Bug: T299280
Change-Id: Ic224d5828f7b7ac0928c44f526c61654ccf3425e
Note how this currently behaves. The user input is
<ref name="… …">
But what we get in the end is
<li id="… …">
This implies that the is decoded and re-encoded with a
slightly different entity encoding. (Note that and  
and   are all the same character.)
Also note how there is only an underscore in the href="…", but the
non-breaking space is gone. This is identical to what happens in
links and headlines. Try for example [[a _a]]. Multiple
underscores, non-breaking spaces, and normal spaces will be
normalized. We just do the same in the id="…" attributes.
Note this fixes only one of the issues listed in T298278.
Bug: T298278
Change-Id: Ia01f2fdd3b3e9ee6aaa9da60ca3386dcd5d6b1a0
This patch makes only sense together with I5a64ac4 where it is split
from. See I5a64ac4 for details.
The idea is that this patch just re-arranges the code without making
any changes to how the code behaves. This leaves a minimal change
behind that's much easier to revert, if needed.
Bug: T298278
Change-Id: Ie78313b7f3ac1ec7bce5ac7512e60a3bb011480a
This patch does two things:
1. The "normalization" function was never only doing normalization,
but also all the necessary HTML encoding. This is now more visible
and split into two separate functions.
2. To make this easier we change the order slightly. Because of this
the normalization step must now consider spaces. Before spaces have
been converted to underscores by escapeIdForLink.
The results are all the exact same as before.
This is split from I5a64ac4 to make that easier to review.
Bug: T298278
Change-Id: I9435a2ddaa21559e29587c58b7523103141467f7
User-options related classes are being moved to the MediaWiki\User\Options namespace in MediaWiki Core; reflect that change here.
Bug: T352284
Depends-On: I9822eb1553870b876d0b8a927e4e86c27d83bd52
Change-Id: I04337d15d32c1e8bdfdcfa272f1750ffecc8e47e
This uses semantics consistent with the DOM spec (and any future
spec-compliant PHP DOM library, like Dodo or a native one bundled
with PHP). It also helps avoid certain classes of errors caused
by conflating empty-string values and missing attributes.
A number of minor bugs have been fixed in the process, mostly
involving the string value "0" (and similar) which are falsey
and thus were sometimes treated the same as a missing attribute.
The API for MediaStructure was slightly tweaked to conform to
this "return null for missing attribute" convention.
No one outside Parsoid appears to use MediaStructure yet:
https://codesearch.wmcloud.org/search/?q=MediaStructure
And Parsoid itself doesn't usethe MediaStructure::get* methods
which were tweaked here.
This patch squashes (and replaces):
Ib272bd2a9a2da06efc4a4d962cd191107a70f84c
Iae47c9f3c80f1642426ef985f1af6b44a9c807fb
Ic3a43516e2a0c9bbd62f9519e5663b545fd975e7
I7c45552a21ba49c76b1fc56f023d7937d9f41340
Ic231a53dba9527c6b811cba990ae35f7fc7bf153
I5209338d221ebf738a505d85fe1a019ea621fcc5
Icb6b13c2b49edb3cd725999f09f8cb7e3a4c0f99
I7eca3e616a66cac5b2a792435889cced2e2c9cd9
Change-Id: I11e7efb546c8cf1aac6b49c3361aacbd4eb5cef1
While this is not really a bottleneck (usually the list of classes
is short) I was still curious. Why create an object, explode a
string into an array (even using a regex) and search the array if
we can do the same with a single regex?
This will be beneficial for other codebases, notably
DiscussionTools.
Change-Id: If9252a97562542e7a4bec29edcc6e8dee0fb8221
Slowly inch toward replacing the plethora of `stdClass` in the
Parsoid port with proper types. The new classes could use additional
documentation of the various properties, but this is a start.
Bug: T226428
Change-Id: I87e73243203145b4067e70b0fa9e30da054fe1ed
* There is really no reason to run this once per nested pipeline
since there is a lot of other processing that is skipped on
nested pipelines.
* Eliminated the 'topLevel' arg to the wtPostProcess extension handler
since that is no longer relevant.
* We may need to explicitly document this in the extension API spec.
Change-Id: Ib2508df736aeab9c649138249228647c00aaaef5
This reverts a very tiny part of Ib3fdc89 from 2 weeks ago. The
reasons are explained in Ib3fdc89. Short version:
* The ->parse() calls have drastic performance implications.
* Allowing wikitext and HTML in this message also makes T321217
worse.
The new message "cite_reference_backlink_symbol" is kept and still
used in the UI. Just not in these two messages any more. This is a
minor redundancy we want to get rid of at some point. But it's not
critical for the moment. This will be done as part of T321217.
Nothing will break on the wikis. Some wikis have customizations for
"cite_references_link_one" and "cite_references_link_many" in place.
This will continue to work as before Ib3fdc89.
Bug: T339973
Change-Id: I933771e3ad67cd530bcf5ee8469cef35ea1070d2
This is a mistake that exists in this codebase for who knows how
long.
Cite mis-uses the messaging system a lot for internal things we still
want to customize somehow, but are not labels that will ever be shown
on the screen. The prefix/suffix messages in this patch are meant to be
part of the HTML in id="…" attributes. Prefix/suffix must be a static
plain text strings. Using e.g. {{GENDER}} or {{PLURAL}} in these
messages is not even possible because there is no $1 parameter to use.
Note how all other similar messages already use ->plain().
A few wikis override these messages, but stick to the plain-text
convention, as they should:
https://global-search.toolforge.org/?q=.®ex=1&namespaces=8&title=Cite.*reference.*fix
This will continue to work.
This has minor performance implications. Fetching these messages is
faster if we can skip transformations.
Bug: T321217
Change-Id: I7969c255fe4ce897e904897081da5f52678721aa
* But, since Parsoid uses different class names, the JS code
that inspects Cite content won't work!
In this specific case, that code looks for .mw-cite-backlink
which Parsoid doesn't add.
* Given the experience with media output changes in core, it might be
prudent to switch over class names to what the Cite extension uses
rather than try to update all the code that are probably referencing
these classes.
Bug: T328695
Change-Id: Ie240898fa9de45f6d9d9821be39e9ec0af9f6bdf
The contents of the domfragment gets migrated to an ol element and
then a query for sups in the ol makes up the nestedRefsHTML string.
Follows-Up: I44abd4f13254bc4512caad85244d523ac19140e7
Change-Id: Ie29d9b1ea7b34121587e77c8693dba79ed166ee1
The WikiEditor extension has a button and some help text that
is only applicable if the Cite extension is enabled. Move
that (with some modifications) to the Cite extension instead.
Bug: T339973
Depends-On: I8256660f9c6886d6764b45735284e00308fc56e5
Change-Id: Ib3fdc897dd3330f69c5832003d4c3cb1e6dba2f3
* Parsoid core cannot know anything about how extensions represent
information in their DOM. We were missing an extension API method
and config that lets extensions specify this information along with
handlers to process embedded content.
* This patch fills that above gap.
* There is a FIXME in ContentUtils::shiftDSR which will be addressed
in followup patches with this new functionality.
Change-Id: If921ca471f25f21671a60c3f796fdf145267364d
* This method isn't used anywhere else outside (per codesearch)
so being bold and renaming without any public notification.
Change-Id: I0c4ec7c4b2afb1d0d3cd1a932e0384bd9626799e
* reads the new attribute extends from wikitext
* saves it into the reference model
* adds a message to the VE popup of an extension as a first demo
* tests will be added in a separate patch
Bug: T247922
Change-Id: If4d309c4678022642f39e21565950dc45e557d47
* leadingWS is always '' in all the uses so far and it originally
came from Cite when we first extracted the extension API.
* It is unnecessarily confusing and serves no real purpose.
Change-Id: I5f2f0f123dc6938735c09b96a42c1923b0a49085
* This builds on the core patch that adds this new method
to core's SiteConfig class.
Bug: T268777
Depends-On: Ia489cd880de3df73d7cd47d1eb01971f2655effe
Change-Id: Ifd41e4275a9344474e48d013e1116b028c7775d7
The algorithm in the grammar is fixed up and the restriction to one
level of nesting is lifted. It was overlooked that parser functions can
be nested. Some of the history of ref-in-ref can be found in 03bf14b0
and d634925.
Bug: T326521
Change-Id: I8e442d41f68133c1b4f16556fedaff6106e233fb
"follow" refs are cloned, which can lead to issues with their
data-objects conflicting with the initial node: if
additional, possibly out-of-order processing is happening, we may end up
accessing/modifying the data bag. In particular, we considered adding a
call to the DOMNormalizer when serializing the reference bodies, and
this was triggering an exception when accessing the DOMDiff of the
cloned node. We are not considering this right now anymore, but cloning
the NodeData may avoid issues of this type in the future.
This patch also introduces a utility to do the clone+clone data bag in a
single method call.
Change-Id: Iccdae82dec81d488433981d764bea539609497eb
* These files were generated with the script in
I3623e42d4cad7975813892a8f0f7765b74a638c5
* These styles mimic the behavior of Language::formatNumNoSeparators
and Language::localizeSeparators used by a couple of methods.
ReferencesFormatter:referencesFormatEntryNumericBacklinkLabel
calls ReferenceMessageLocalizer::localizeDigits which calls
that core Language method.
FootnoteMarkFormatter::linkRef calls localizeDigits as well.
- '.reference a*' CSS rules mimic linkRef localization
- 'span[rel="mw:referencedBy" ]*' rule mimics localization
of referencesFormatEntryNumericBacklinkLabel
* Overrode all hand-crafted language CSS files from previous
patches. The content of some of those hand-crafted CSS files
will end up in those wikipedia's Common.css -- specifically
for es, fr, sv wikipedias.
sv and es have non-canonical formatting strings that aren't
handled by the script right now and so leaving them behind.
I need to look at fr output more carefully, so leaving that
behind as well. But the generated ones are more accurate when
combined with the wiki-specific CSS files genreated by the other
script that processes site messages.
* Some files could potentially be removed by looking at
language fallback chains, but the redundancy is not a
problem for now.
* Tweak the resource loader script to use "_" instead of "-"
when looking for a resource file.
Bug: T156350
Change-Id: I000b4538bf2be681b85df5813efed083458a4281
MWParsoid\ParsoidServices depends on the deprecated
\MediaWiki\Parser\Parsoid\ParsoidServices in core; it is not needed
any more in MW 1.40 so it has been removed.
Change-Id: I2c4fd2e9ea2b5e2751074f1b9705e46436a32131
The ''outputHasCoreMwDomSpecMarkup' option in the extension registration
indicates whether the output of an extension complies to the MediaWiki
DOM Spec and should be processed as such.
The initial goal of this setting is to enable the DOMNormalizer pass for
Parsoid HTML generated in extensions, as needed by the implementation of
T309024.
We setup "Cite" as an example of configuration.
Bug: T309024
Change-Id: I26182c67ea17a4aa59414856d5f4614f2cd06c46
The 'rel' attribute can be used for multiple values in the same way that
the 'typeof' attribute can. In this patch, we allow for that by
providing methods to handle these multivalues and by refactoring the
code that would only allow for single values.
Bug: T186241
Change-Id: I625b121ae77c06c519c3b578583d41dc220e3294
The best practice for message keys is to use dashes, not underscores.
This codebase is quite old and traditionally uses underscores. I
think we can make it flexible enough to work with both.
Required for Ie64f4ab.
Change-Id: I6f0584299a4f279ed929784927392eb0f72cbc80
Extensions using Phan need to be updated simultaneously with core due
to T308443.
Bug: T308718
Depends-On: Id08a220e1d6085e2b33f3f6c9d0e3935a4204659
Change-Id: Iebc5768a3125ce2b173e9b55fc3ea20616553824
These changes are a result of adding Parsoid integrated testing support
in core.
* Some tests got new html/parsoid sections.
* Some tests got their html/parsoid output upgraded to remove bogus HTML.
* Since we are not treating the test runs to be in integrated mode,
I removed some comments that only pertained to a test run in
standalone mode.
* There are 8 tests that are failing in integrated mode as well and
I've tagged 5 known ones with T307741. The other 3 failing ones are
the responsive reference tests that have a threshold value set.
Those failures will need investigation and require additional changes
to the core parser test runner to pass through the config to Parsoid.
Change-Id: I370d57d45cf126f71b3666fb493a887faf6b8e0d
When a reference is inserted in a link, Parsoid generates nested links,
which would break browser rendering. In some cases where the generated
HTML is reprocessed in later stages of Parsoid (as is the case in
T301293), it can lead to internal breakages.
In this patch, we propose to hoist references markers (and their <sup>
tag) included in links outside of that link, as next sibling to the link.
It avoids the vast majority of link nesting due to that situation, with
the following side effects:
* the <sup> tagging is now maintained around the marker tag,
* if a ref tag is in the middle of a link, it gets moved to the end of
the link (in the legacy parser, it stays where it is, but the end of the
link gets de-linked).
The selser test failures are consistent with the expected behaviour - wt2wt
does not round-trip correctly, leading to selser failures.
Bug: T301293
Change-Id: Ia39483c2112b1356e14a310fbb48baed946b5caa
Implements mocking for the #tag parser function for ref tags.
The ref-in-ref linting cycles tests from 04aa4be can now be restored.
This shows that after I1b598bd359b900d1b89abf5d8105a5d131aea3d1, the
protection added in 04aa4be is no longer necessary, because we only lint
the content where it's defined.
Bug: T237463
Change-Id: I4059e32b9bea8cdc23d2112812c3f7e167e47399
Follow up to I1b598bd359b900d1b89abf5d8105a5d131aea3d1
This also lints the html stashed in data-mw of mw:Extension/ref, when
named references have redefinitions.
The fast fail for linting references with errors is removed since it's
no longer necessary after the work in T51538.
Bug: T214994
Change-Id: I2431b4782339a1ac41c49f7ca0ad3480c0b13bad
This change will fix the crasher from T301293, since all the necessary
information to locate the ref contents is contained in the first
encapsulation wrapper node, we therefore don't need to traverse into it
and potentially be tripped up by the node being closed early for having
content model violations.
By using the linkback id from the href, we're potentially linting the
content multiple times.
By using the id from the data-mw, we're only linting the content
specified by the specific ref (with the slight caveat that if two named
refs define exactly the same content, they share an id).
Note that if named refs have multiple definitions, and hence the content
ends up in data-mw, we aren't yet linting it, that's T214994.
However, by not using the linkback ids from the href, we'll need to
traverse the html that mw:Extension/References have stuffed in their
data-mw if we want to lint references defined in the references tags
themselves (see the commented out test). This has the benefit though of
not running into the issue described in the References::lintHandler
(ie. not having the right tplInfo while traversing the content).
Bug: T301293
Bug: T214994
Change-Id: I1b598bd359b900d1b89abf5d8105a5d131aea3d1
Content Translation can lead to Cite references to nodes that aren't present
in the translated document. For now, I've suppressed these log entries in
Parsoid's logstash dashboard. But, there is no reason to continue emitting
these at error level given the large volume of CX pages and hence a large
volume of these non-actionable log entries.
Change-Id: I8df7e722203d7b866d987d626215bcd53b945d60
Note how only the HTML5 mode behavior changes, but nothing in
legacy mode.
Also note this does not 100% fix the issue. The esample with a
non-breaking space is still broken. But it's already much better
than before.
Bug: T298278
Change-Id: Idf50dad4219ff4c594a0cc15f63cb10fdac5ffb7
* Remove the overhead of serializing and then re-parsing client-side,
instead assign it directly as native object literal.
* Move code for array slicing to PHP.
Change-Id: Iedcc8d57d3bddd3fa32a78b4e7ecc25615d94277
Rather than depending on a separate module with one line of generated
JS code, generate it as a prepended statement to the same module.
Should be a no-op.
Depends-On: I809951d34feb2dbd01b7ae0f4bd98dac7c3f6fe2
Change-Id: I5886bf9f82025048976b7750e8cb751681021fb4
Use @property to provide the types of undeclared variables to Phan and
PHPStorm, as in my NodeData patch. Declare $dp->tmp since it is
commonly used and does not affect the JSON serialized output since it is
always stripped.
I omitted the constructor, instead of following the suggestion in the
massageLoadedDataParsoid doc comment which proposed injesting a
JSON-like data structure in the constructor. I thought it would be more
efficient to have the initial property assignments inline in the calling
code. This means breaking up many object cast expressions into
individual assignments.
In IncludeOnly, the coalescing null operator was only handling the case
where $start->dataAttribs was unset, which seems unlikely. I made it so
that it checks whether $start->dataAttribs->tsr is unset.
I added strongly typed clone() methods, to preserve type information for
static analysis.
DataParsoid is the type of the data in both the DOM and in tokens. To
simplify the changes to the Token hierarchy, I removed the duplicate
definitions of the public properties $attribs and $dataAttribs.
Change-Id: I16172083e7e9bcb94601d1d6862d1d202a7e3660
* Most of these are remnants from the Parsoid/JS codebase.
* This change follows the pattern we've been using everywhere
since the port from JS->PHP.
* Also reduces instruction count by about 0.2%.
Change-Id: Ibf21104f6722c34299f03e303dc3401bf053a751
Review regex usage, and use an alternative where possible, to improve
performance.
* Add PHPUtils::stripPrefix() and PHPUtils::stripSuffix(). Benchmark in
doc comment.
* /foo/ -> str_contains()
* /^foo/ -> str_starts_with()
* /^f/ -> ($s[0] ?? '') === 'f'
* /foo$/ -> str_ends_with()
* /^(foo|bar)$/ -> in_array(), benchmark suggests 10x improvement
* preg_replace(/foo/) -> str_replace()
* preg_replace(/^[abc]/) -> strspn(), benchmark suggests 3x improvement.
Curiously, it is faster without a limit for short input strings,
although a limit presumably adds robustness.
* preg_replace(/[abc]+$/) -> rtrim()
* preg_match_all() -> substr_count()
* In DOMUtils::hasTypeOf(), use explode() instead of a regex. Validated
by a benchmark.
* In DOMUtils::addTypeOf(), stop normalizing adjacent spaces. This
allows us to use implode(explode()) without a filtering loop. The
patch to Ext/Cite/References.php was to remove spaces added by this
change. The parserTests.txt changes were a consequence of the
References.php change.
* In LinkHandlerUtils::getHref() I allowed a single bare slash to be
counted as a path-absolute URL since I think that was the intention of
the original code.
* In LinkHandlerUtils::getLinkRoundTripData() I captured the portion of
interest from the previoulinkHandlers regex instead of running a
second regex.
* LinkHandlerUtils::linkHandler() had the regex
/^mw:WikiLink|mw:MediaLink$/ which I think was a bug, missing
parentheses. I fixed the bug.
The margins are pretty tight for a lot of these. Using polyfills for
str_contains() etc. might change the conclusion.
Also:
* In DOMUtils::matchTypeOf(), avoid calling hasAttribute().
getAttribute() is documented as returning an empty string if the
attribute does not exist.
Change-Id: I8d7bdf1bccc869b4dc17058a5822ef34968471e6
Follow up to 47dd898
Also renames a variable to be consistent in the two places we get
contents for the ref.
Change-Id: I13e61b8911ff16549fbb0888b9c3313ed5e7701e
Follow up to 47dd898
Fixes the test case found in rt,
php bin/parse.php --domain ceb.wikipedia.org --pageName "Martin Van Buren" --offsetType ucs2 < /dev/null
The offsetType is necessary so that the ConvertOffsets pass runs. The
crasher here is because the embedded html still contains the sealed ref
fragments because we've stored the unprocessed html.
Change-Id: Ic1e1c3e54433bf6d7574420c2eade1349261de0b
Restores linkbacks for ref-in-ref.
Follow up to 568034a where it's noted that it's fine to maintain
linkbacks for ref-in-ref, as long as the ref isn't a named ref that's
trying to redefine the contents for that name, in which case we embed
the contents.
A test case for this can be,
```
<ref name="hiho">off to work</ref>
{{#tag:ref|<i>we go <ref name="ohno">ohno</ref></i>|name="hiho"}}
{{#tag:ref|<i>we go <ref name="ohno2">ohno2</ref></i>|name="test"}}
```
The linkback to #cite_ref-ohno2_3-0 is present while continuing to
suppress the dangling linkback to #cite_ref-ohno_2-0, since that's in
embedded content. On master, both linkbacks are unnecessarily
suppressed.
Bug: T289331
Change-Id: Ifcf7464e86a4408f5dd9e2fd6d3aa47a0670ca49
This will be helpful in a subsequent patch where we make use of that
data while processing refs in refs. Content differing implies that
we'll be embedding it for roundtripping, rather than putting in the dom.
Change-Id: I7bd1d4c503fc58f862960bec82ca514fc29d7eff
This moves determining if we already have a reference created for a
named ref outside of that function, which is helpful for making use of
the cached html for that ref earlier.
Change-Id: Ie416bd95b980f9f95111d7e420945f40e2ada747
The standard type for these returns is NodeList and HTMLCollection, which
are almost *but not quite* the same as an array. In two places we got a
little complacent and assumed our non-standard DOMCompat workarounds would
always return arrays. Tweaked the types of DOMCompat to report that they
return an `iterable`, which is a PHP7.1 "pseudo-type" that unifies
arrays and \Traversable types like HTMLCollection/NodeList. This
allows phan to catch places where we slip up and assume an array type
return.
It does introduce a new wrinkle, though, since there is no simple way
to turn an iterable into an array. We're using a simple
`iterable_to_array` helper function for this.
Change-Id: I35bdeb3afa30ef5182e71733a0a606aadcafb435
In PHP's DOM extension, one of the legacy bugs is that
DOMNode::getAttribute() can never return `null` (to indicate that the
attribute is missing), instead it returns an empty string in that
case. This isn't (modern) spec-compliant behavior (it's a leftover
from ancient times) and we had to watch this carefully when porting
from JS.
In the time since the port, we've written new code and embedded this
assumption that DOMNode::getAttribute() will never return null into
the new code we've written. Fix this. Always use `getAttribute(...)
?? ''` (unless we're just doing an equality test against a non-empty
string, or the code is preceded by a `hasAttribute` test) so that our
code will work whether or not getAttribute returns null for a missing
attribute.
Change-Id: If33200e1053b2dd79abb5dfb3808c05ff3a0bbba
Mostly comments along the lines of "{classname} constructor"
in the doc block for the __construct method.
Change-Id: I67ffe070985dc75a5d817b1b5ac97b529d7ab4b8
I tried many things, but wasn't able to reproduce the error
described in T283755. What probably happens goes like this:
* Somehow $this->refs[$group] is initialized, but
$this->groupRefSequence[$group] is not.
* There are not many places in the code where this can
happen. There are a few suspicious lines in rollbackRef(),
but they are all guarded. The only problematic place is in
appendText().
* This problematic line is only called for <ref> in
<references>.
* Somehow a <ref> is valid enough to make it to appendText(),
but not valid enough to make it to pushRef().
* The next time another <ref> is added to the same group, it
appears like the group already exists ($this->refs[$group]
is set), but $this->groupRefSequence[$group] is missing.
I was unable to find a wikitext example that would behave like
this.
This patch just makes sure the initialization is done but
doesn't care why it was missing. The following code is fine
with an existing ref that contains nothing but text (which is
how appendText() leaves it behind).
Bug: T283755
Change-Id: I36ac56ef6ed98676a3e8f430a796826351a5f4e9
