Three new checks are now applied to user signatures in preferences:
* Disallow invalid HTML and lint errors (T140606)
Since 15e0e9bb4b we can rely on Parsoid to check the signature for
lint errors. (The old PHP Parser doesn't have this capability.)
Most importantly, this will disallow unclosed HTML tags. Unclosed
formatting tags like `<i>` (and also wikitext markup like `''`)
could affect the entire page with the bad markup.
New configuration variable $wgSignatureAllowedLintErrors is added
to allow ignoring some errors. The default value ignores the
'obsolete-tag' error (caused by HTML tags like `<font>` and `<tt>`.)
* Require a link to user page, talk page or contributions (T237700)
Various tools don't work correctly when such a link is missing. For
example, Echo notifications are not sent, DiscussionTools will not
allow replying to these comments, English Wikipedia's SineBot treats
these comments as unsigned.
Such requirement has been present for a long time in many Wikimedia
wikis' policies, but it was not enforced by software.
* Disallow "nested" substitution in signature (T230652)
Clever abuse of "subst" markup and tildes allows users to save edits
containing wikitext in which substitution occurs again when the page
is next saved. Disallow this in signatures, at least.
New configuration variable $wgSignatureValidation is added to control
what we do about the result of the validation described above. The
options are:
* 'warning':
Only displays a warning near the field on Special:Preferences if
the current signature is invalid. Signatures can still be changed
regardless of validity and will be used when signing comments.
* 'new':
In addition to the above, if a user tries to change their signature,
the new one must be valid. Existing invalid signatures are still
used when signing comments.
* 'disallow':
In addition to the above, existing invalid signatures are no longer
used when signing comments.
Bug: T140606
Bug: T237700
Bug: T230652
Change-Id: I07c575c2d9d2afe7a89c4847d16ac044417297bf
Note there is another line of code (line #96, as of now) where the
$this->size property is increased *before* the two $this->internals
and $this->interwikis arrays are increased. Just do the same here.
Change-Id: I15f9e438706d75323ec17cb92e933f600701f9b8
This is a series of extremely basic, trivial transformations that
don't change any behavior. The goal of this patch is to make the
code less surprising and less cluttered.
In detail:
* Remove an unused property:
https://codesearch.wmflabs.org/search/?q=tempIdOffset
* Add a strict "Parser" type hint. Note this code would fail anyway
if that property would not be a Parser.
* Avoid count() if we don't need to know the actual number, just if
it's empty.
* Inline a few single-use variables.
Change-Id: Ic76cc3984462b1b7700bbc675adaca8fc8219152
Deprecating something means to say something nasty about it, or to draw
its character into question. For example, "this function is lazy and good
for nothing". Deprecatory remarks by a developer are generally taken as a
warning that violence will soon be done against the function in question.
Other developers are thus warned to avoid associating with the deprecated
function.
However, since wfDeprecated() was introduced, it has become obvious that
the targets of deprecation are not limited to functions. Developers can
deprecate literally anything: a parameter, a return value, a file
format, Mondays, the concept of being, etc. wfDeprecated() requires
every deprecatory statement to begin with "use of", leading to some
awkward sentences. For example, one might say: "Use of your mouth to
cough without it being covered by your arm is deprecated since 2020."
So, introduce wfDeprecatedMsg(), which allows deprecation messages to be
specified in plain text, with the caller description being optionally
appended. Migrate incorrect or gramatically awkward uses of wfDeprecated()
to wfDeprecatedMsg().
Change-Id: Ib3dd2fe37677d98425d0f3692db5c9e988943ae8
There's already a thing called magic words, and this is not it. These
things are called variables. There are many usages of this term in the
source. The term was introduced by Lee in 2002: originally
OutputPage::replaceVariables() contained only this functionality.
I introduced the term "magic word", meaning a localizable keyword.
Localizable keywords are an abstraction not limited to this use case.
"Magic variables" is a neologism, but I suppose it is permissible, since
it disambiguates. Whereas calling a variable a magic word conflates rather
than disambiguates.
Fix terminology in magicword.md and update the examples.
Change-Id: I621c888e3790a145ca9978f6b30ff1a8f685b64c
The future Parsoid parser will not support this, and it appears to be unused.
It could be reimplemented as an extension tag once it is removed from core.
Code search:
https://codesearch.wmflabs.org/search/?q=allowimagetag&i=fosho&files=&repos=
Bug: T254802
Change-Id: I1b532a7a8794766f8df6fdf375a6ffd78fee94e5
Per-parser configuration is discouraged; use sitewide configuration instead.
Code search:
https://codesearch.wmflabs.org/search/?q=setEnableImageWhitelist%7CsetAllowExternalImages%7CsetAllowExternalImagesFrom&i=nope&files=&repos=
The ultimate goal here is to refactor the image filtering functionality into
an extension and move it out of core, so that it can be used by both
Parsoid and the legacy parser in the same way. We may add back per-parser
customization of the filtering, but the API will probably look different.
Deprecate the existing ParserOptions-based mechanism, which code search
indicates almost no one is using.
Bug: T254802
Change-Id: Ib4a59bbae10cfc924c0290948330d93e02de9ed0
Clean up some technical debt; use MutableRevisionRecord instead of
manually constructing a Revision from an array, remove last uses of
RevisionStoreDbTestBase::revisionToRow and remove the method.
Each file can be reviewed separately (except that the removal of
revisionToRow depends on replacing its usage)
Bug: T246284
Change-Id: I0bdc069b21a5c41ef8f9e972c5b17ff189d4a741
The `false` return has been the source of persistent bugs (T253725,
T251952); lets nip this pattern in the bud before we release these new
APIs.
It would be nice to fix Parser::statelessFetchRevisionRecord() as well,
but that was released in 1.34, so it's not quite as easy to change.
Change-Id: I05a968e3dfb660d0709a6417d1d53a1d08ed4818
A terminating line break has not been required in wfDebug() since 2014,
however no migration was done. Some of these line breaks found their way
into LoggerInterface::debug() calls, where they mess up the formatting
of the debug log.
So, remove terminating line breaks from wfDebug() and
LoggerInterface::debug() calls.
Also:
* Fix the stripping of leading line breaks from the log header emitted
by Setup.php. This feature, accidentally broken in 2014, allows
requests to be distinguished in the log file.
* Avoid using the global variable $self.
* Move the logging of the client IP back to Setup.php. It was moved to
WebRequest in the hopes that it would not always be needed, however
$wgRequest->getIP() is now called unconditionally a few lines up in
Setup.php. This means that it is put in its proper place after the
"start request" message.
* Wrap the log header code in a closure so that variables like $name do
not leak into global scope.
* In Linker.php, remove a few instances of an unnecessary second
parameter to wfDebug().
Change-Id: I96651d3044a95b9d210b51cb8368edc76bebbb9e
Although it's true that Parsoid doesn't (yet) support this hook, and
the $parser object referenced in the hook is likely going to be changed,
this is a hook added in 1.35 (eb6c5f70d9)
to replace use of an even worse hook. So let's keep the lesser of the
evils, at least for now.
Bug: T236809
Change-Id: I8f866c3b9f1fc51848cfe9364635112371d18e3e
Migrate all callers of Hooks::run() to use the new
HookContainer/HookRunner system.
General principles:
* Use DI if it is already used. We're not changing the way state is
managed in this patch.
* HookContainer is always injected, not HookRunner. HookContainer
is a service, it's a more generic interface, it is the only
thing that provides isRegistered() which is needed in some cases,
and a HookRunner can be efficiently constructed from it
(confirmed by benchmark). Because HookContainer is needed
for object construction, it is also needed by all factories.
* "Ask your friendly local base class". Big hierarchies like
SpecialPage and ApiBase have getHookContainer() and getHookRunner()
methods in the base class, and classes that extend that base class
are not expected to know or care where the base class gets its
HookContainer from.
* ProtectedHookAccessorTrait provides protected getHookContainer() and
getHookRunner() methods, getting them from the global service
container. The point of this is to ease migration to DI by ensuring
that call sites ask their local friendly base class rather than
getting a HookRunner from the service container directly.
* Private $this->hookRunner. In some smaller classes where accessor
methods did not seem warranted, there is a private HookRunner property
which is accessed directly. Very rarely (two cases), there is a
protected property, for consistency with code that conventionally
assumes protected=private, but in cases where the class might actually
be overridden, a protected accessor is preferred over a protected
property.
* The last resort: Hooks::runner(). Mostly for static, file-scope and
global code. In a few cases it was used for objects with broken
construction schemes, out of horror or laziness.
Constructors with new required arguments:
* AuthManager
* BadFileLookup
* BlockManager
* ClassicInterwikiLookup
* ContentHandlerFactory
* ContentSecurityPolicy
* DefaultOptionsManager
* DerivedPageDataUpdater
* FullSearchResultWidget
* HtmlCacheUpdater
* LanguageFactory
* LanguageNameUtils
* LinkRenderer
* LinkRendererFactory
* LocalisationCache
* MagicWordFactory
* MessageCache
* NamespaceInfo
* PageEditStash
* PageHandlerFactory
* PageUpdater
* ParserFactory
* PermissionManager
* RevisionStore
* RevisionStoreFactory
* SearchEngineConfig
* SearchEngineFactory
* SearchFormWidget
* SearchNearMatcher
* SessionBackend
* SpecialPageFactory
* UserNameUtils
* UserOptionsManager
* WatchedItemQueryService
* WatchedItemStore
Constructors with new optional arguments:
* DefaultPreferencesFactory
* Language
* LinkHolderArray
* MovePage
* Parser
* ParserCache
* PasswordReset
* Router
setHookContainer() now required after construction:
* AuthenticationProvider
* ResourceLoaderModule
* SearchEngine
Change-Id: Id442b0dbe43aba84bd5cf801d86dedc768b082c7
This reverts commit c45ccd7ca8.
Reason for revert: Assuming that I6af7aeabbba fixes the real issue.
Change-Id: Ie1fc595a18e54f0c29b43740039cd7114d8e071e
In brief, the BlockLevelPass looks at opening and closing tags on a line
to determine whether it should do paragraph wrapping. The blockElems
want to stop wrapping when opened and start again when closed. The
antiBlockElems want the opposite, to start when they're opened and stop
when closed. "table" is a blockElems and "td"|"th" are anitBlockElems
so that content found in the interstitial spaces of tables are never
paragraph wrapped.
That means that, to date, "caption" elements are always found in a place
where paragraph wrapping is always suppressed and so adding them to that
set won't change any test results. However, a new test is added to spec
out this behaviour.
In the legacy parser, "captions" are always found in the right place
because handleTables runs at an earlier stage. In Parsoid, however, the
treebuilder is relied on to close table cells [0] so when we get to the
token stream paragraph wrappping pass, "caption"s are found in table
cells and therefore get wrapped, even though the treebuilder is about to
be induced to close the cell before opening the caption.
Therefore, in Parsoid, the fix would require us to make captions always-
suppressing to match the legacy parser behaviour. Thus, this change
here is just to keep these lists [1] consistent between the two
parsers.
[0] 5e11a3f390/src/Wt2Html/TokenizerUtils.php (L138-L151)
[1] 5e11a3f390/src/Wt2Html/TT/ParagraphWrapper.php (L71-L78)
Bug: T210647
Change-Id: I8ccefd69d47dca740f50924b235dffa3873d1f99
This newly-added method returns `false` on error; the caller expects
it to return `null`.
Bug: T253725
Followup-To: If36b35391f7833a1aded8b5a0de706d44187d423
Change-Id: I6af7aeabbba9f95338497026fd08d9ae23f75c22
This behavior has been deprecated and with a tracking category since
1.28. Time to remove the temporary parameter added to
Sanitizer::removeHTMLtags() and (finally) tweak the behavior to match
HTML5.
Bug: T134423
Change-Id: I5c725175d05854139c95a2b3d8d35ff63cb6707b
Reason for revert: issue arose again when deployed with wmf.34
Partial revert: keep the intended fix in Parser.php, revert
removal of fail-safe logic in CoreParserFunctions.hp
This reverts commit 2712cb8330.
Bug: T253725
Change-Id: I06266ca8bd29520b2c8f86c430d0f1e2d5dd20c0
This is a follow up to I3eae3719ab8fb50b7996d4fd8a9fa0d5ca250023 where
it was moved below doBlockLevels.
This puts it next to the other call to the sanitizer and aligns it
closer with the idea of a post-processing pass in Parsoid.
Bug: T197879
Change-Id: I8ba4934c01a24d53d4871b8efa1e9cf737ba9ebd
