It misrepresents the users contribution to show empty text for a
revision when in fact the revision contained some text which we later
lost.
Also, errors from SqlBlobStore::fetchBlobs() did not stop a cache entry
from being written, so a subsequent cache hit would show the bad
revision as empty.
So, in Storage:
* Add BadBlobException, which is thrown by the Storage layer to
indicate that a revision is marked as bad.
* Have SqlBlobStore::getBlobStore() return an error for bad blobs
instead of an empty string.
* Duplicate the check for flags=error into SqlBlobStore::expandBlob().
This avoids an unnecessary cache fetch, and avoids making
decompressData() throw on error, which would be a b/c break.
* In SqlBlobStore::getBlob(), suppress the cache when there was an
error.
In Revision:
* Add BadRevisionException, to wrap BadBlobException in the Revision
layer.
* Return null from RevisionRecord::getContent() on a broader set of
errors. Make it mostly non-throwing.
* Add RevisionRecord::getContentOrThrow() which returns a non-nullable
Content.
* Note that SlotRecord::getContent() returns a non-nullable Content so
now throws in more cases.
In the UI:
* In Article::view(), catch the exception and show an error message.
* In DifferenceEngine, catch the exception and make a suitable error
message available via getRevisionLoadErrors(). In the diff page, show
the error message in a box.
* In ApiComparePages and the legacy rvdiffto, show a warning.
* In RawAction, show a 404 by analogy with other error cases.
* In EditPage, there was already handling for $content=null with an
appropriate error message (missing-revision-content). But having
$this->textbox1 = null caused PHP 8.1 deprecation warnings, so I fixed
that.
* In EditPage undo, there was already handling for null content, but I
improved the error message: "does not exist or was deleted" seems more
appropriate than "conflicting intermediate edits".
Change-Id: Idd1278d6d756ef37d64addb7b5f3be30747ea603
In 81f69c2, I have only removed the callers. The method is
now dead code and should have been removed as well.
Change-Id: I556ed5398435f4d3a5f400e07e4819dd16e0df06
When setting causeAgent for jobs, be specific.
When creating parserOptions, call setRenderReason.
This allows us to capture statistics on what actions cause jobs,
renders, and cache writes.
Change-Id: Iaef64beadce1489bebb6bb00855c3ba6013a29d8
PHP 8.1 adds single quotes to the list of encoded characters when
htmlspecialchars() is called with no flags. That's usually harmless,
but this instance breaks an extension test. So use ENT_NOQUOTES which is
optimal for text which is not inside a tag or attribute.
Bug: T322099
Change-Id: I3a2c385771b3f00a38a238bf745bd2997be60a9c
This patch only adds and removes suppressions, which must be done in the
same patch as the version bump.
Bug: T298571
Change-Id: I4044d4d9ce82b3dae7ba0af85bf04f22cb1dd347
Mainly, document some parameters as non-empty-array so that phan knows
the list of arguments won't be empty when unpacking.
In EditPage, account for hooks potentially unsetting the copyright
notice.
Also rewrite some code in LogPager, so it's hopefully easier for phan to
understand what's going on.
Change-Id: Ic0638571554424098d0743db32dd46723a08e103
We rely on the default settings for `skin` and `injectTOC` set by
OutputPage::addParserOutputText(), and so no longer need to explicitly
initialize these from the skin options.
Bug: T317333
Depends-On: Ica30569efbb5730eff5b807e8fc34beb2e13e74f
Change-Id: I5b8ecd1abf87e66bb75d7c26790f9e7c1b3a6da3
Previously, OutputPage::isRevisionCurrent() would check a replica
database, potentially different from the database used to fetch the
revision being displayed, according to OutputPage::getRevisionId().
Now the data for both functions is provided in the same way.
Bug: T314684
Change-Id: I266d643d1eed931df346889f43d72d42e5f4a3ba
Using the same rationale as for its parent class, Diff: ideally there
should be some kind of factory for diffs, but currently, there isn’t.
Reuse the same task ID for the todo comment (T257472), as we think it’s
appropriate for both “diff” classes, Diff and WordLevelDiff.
Note that the TwoColumnConflict extension already uses this class as if
it were newable.
Bug: T311224
Change-Id: If2553d6ebc96636b83df8f58e0420dd7eb6ba5af
Co-Authored-By: Michael Große <michael.grosse@wikimedia.de>
This edition brought to you by:
grep -ERIn $(grep -o "'[A-Za-z0-9_]*'" includes/MainConfigNames.php | tr
"\n" '|' | sed 's/|$/\n/') includes/
I only corrected a fraction of the results provided by that command. I'm
submitting the partial patch now so it doesn't bitrot.
Bug: T305805
Change-Id: If1918c0b3d88cdf90403921e4310740e206d6962
Make phan stricter about null types by setting null_casts_as_any_type to
false (the default in mediawiki-phan-config)
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together
Bug: T242536
Bug: T301991
Change-Id: I0f295382b96fb3be8037a01c10487d9d591e7e01
The functions returning null or the class property is set explict null.
Some function should not accept null or return null.
Found by phan strict checks
Change-Id: Ie50f23249282cdb18caa332f562a3945a58d86ff
The functions returning null or the class property is set explict null
Found by phan strict checks
Change-Id: I4a271093fb6526564d8083a08249c64cb21f2453
The purpose of all these changes is not only to make the code
shorter, but to make it more readable.
Notable:
* Fix wrong type hints.
* The `foreach ( $roles )` loop was processing everything twice.
* Inline preg_replace_callback() callbacks.
Change-Id: I2b8146946ab6966cc6f047556a0a898624af181a
Default comment is added to changelist rows that have no comment/edit
summary. This affects history and diff pages as well contributions page.
The default comment is hidden by default.
Bug: T298645
Change-Id: I2982a69886a90797b2a658df13db9780e88b871c
For Special:Diff/prev/1234567890 the message is
2 revisions of this difference (0 and 1234567890) were not found.
The 0 is not helpful
Change-Id: I6bada64c8cffb4e653ea7a46c31fb014fd6b862f
This reverts commit ef458e8948.
Reason for revert: Causes page tabs to disappear on Special:WhatLinksHere.
Bug: T297744
Change-Id: I0ee282a9f7a5a9b2cfdc3261d800d9e27eaf977e
This name is consist with the rest of the setter and getter methods
in ParserOutput. Renamed the methods in OutputPage, ImageHistoryList,
ImageHistoryPseudoPager, and ContribsPager as well for consistency;
it also makes chasing down lingering references in codesearch easier.
Soft-deprecated the old name for 1.38. Hard-deprecation will follow,
but there are a number of users in production that should be chased
down first.
Code search:
https://codesearch.https://codesearch.wmcloud.org/deployed/?q=(allow%7Cprevent)Clickjacking&i=nope&files=&excludeFiles=&repos=
Bug: T287216
Change-Id: I9822c60c180d204bd30cb4447a1120155d456da4
Nothing in the code expects things to be on the left/right, it was just
a naming convention. However, it caused some issues because CSSJanus
flips the attributes in the stylesheet for RTL languages.
So just use more descriptive names that also avoid issues with RTL
languages.
Bug: T290731
Change-Id: I8a383cd1e3981dc8a826ff60eee523d9f71d1d3d
This helps phan to detect unreachable code and also impossible types
after the functions.
It helps phan to avoid false positives for array keys
when the keys are checked before
Bug: T240141
Change-Id: I895f70e82b3053a46cd44135b15437e6f82a07b2
This is inspired by the github approach, with some adjustments,
simplifications and improvements. This approach should be supported by
all grade C browsers, except for Firefox 27-51. As such, it seems the
best solution until we can use the grid layout (T270775).
The boilerplate code for the new module was shamelessly stolen from
Ied858465c2e938828fc146880827acb6aa15fdd6.
Also noting here for posterity: this will NOT work in WMF servers (beta
or prod), since we use wikidiff2 to generate diffs there.
Bug: T285956
Change-Id: I0cb5f10254af78043b0f6250f7695c6b962fbf6b
Updates for the removal of the Revision class itself
and the various methods/hooks/variables removed in the
process, including:
- Update some documentation removing most references
to the Revision class and updating the MCR migration
notes to reflect the past tense for Revision methods.
- Change some capitalization from "Revision" to "revision"
to make it clear comments are about revisions in general,
not the Revision class in particular.
- Minor code tweaks including removing unused variables that
were around for the old hooks that were removed, and
removing the use of DeprecatablePropertyArray where no
longer needed for anything.
- Fix incorrect documentation for PageUpdater::getStatus(),
the status value changed a while ago to have revision-record
in addition to revision, and recently to only have the
revision-record, but ironically PageUpdater was never updated.
- Removed Parser::$mRevisionObject, used to be a Revision object
and was deprecated in 1.35, missed earlier because it was no
longer being set to Revision objects, always null.
- Add RevisionRecord typehints in DummyLinker to match those
in the corresponding Linker methods
This should be a no-op in terms of functionality.
Bug: T247143
Change-Id: I03bbb94fc29085855448780b1a5ad9063911ecc4
This patch adds and updates documentation. The most significant
changes are:
* getOrig() and getClosing() can return false. However, this is
only possible for add and remove operations. These have only
one side.
* The constructors of the subclasses are not meant to accept
false, and are in fact never called with false. For the same
reason. Only add and remove operations are allowed to miss
one of the two sides.
Change-Id: I9f45f34945e0297e1ea8d3e8ff9e9c53e60e7706