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
* This now aligns with Parsoid commit 0fab92ba453d424aedeadaaa9e1514c42bbd94d1
* Disabled the newly added tests because that Parsoid fixes for the
tests haven't been released to vendor to let CI pass these tests.
* Re-enabled a previously disabled test.
Change-Id: I4ab87d2d486b7a1fef652c50c4f1e79ddfe83ce6
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 reverts commit b163add15b.
Reason for revert: This was my mistake. I forgot that reverting this
would break Parsoid CI once the Parsoid Cite patch merged. So, I have to
wait till the Parsoid Cite change is released to vendor before I sync
the test change here.
Change-Id: Icaecee1e56907980681aae01be377b6906bd93a6
* 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
For example, use convenient upstream methods, and generally make the
test setup a bit more readable.
Bug: T353227
Change-Id: Ifab71041fcc3f804315793ca7b783f84829c7a0f
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
I wanted to make this a unit test but it turns out the
Sanitizer::safeEncodeAttribute() calls currently make this
impossible.
Bug: T353269
Change-Id: I5266e7b8b67db1c812dc9e4675d0c079ab1f9a40
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
> We lose useful coverage and spend valuable time keeping these tags
> accurate through refactors (or worse, forget to do so).
>
> I am not disabling the "only track coverage of specified subject"
> benefits, nor am I claiming coverage in in classes outside the
> subject under test.
>
> Tracking tiny per-method details wastes time in keeping tags
> in sync during refactors, and time to realize (and fix) when people
> inevitably don't keep them in sync, and time lost in finding
> uncovered code to write tests for only to realize it was already
> covered but "not yet claimed".
https://gerrit.wikimedia.org/r/q/owner:Krinkle+is:merged+message:%2522Widen%2522
Change-Id: Iafa241210b81ba1cbfee74e3920fb044c86d09fc
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
