We’re appending at least a closing brace to the source code, so if that
code ends with a line comment, we need to have a line break between the
end of the code and our appended brace.
Bug: T286996
Change-Id: I4e93b10f31445f6dab76c1614e546fb41fd6c0f6
Gated behind the flag $wgParserEnableLegacyMediaDOM. The scattershot
usage of it is a little unfortunate but isn't expected to live very long
so maybe that's acceptable.
Further details can be found at,
https://www.mediawiki.org/wiki/Parsing/Media_structure
Bug: T51097
Bug: T266148
Bug: T271129
Change-Id: I978187f9f6e9e0a105521ab3e26821e36a96b911
There is a fallback in Module->getConfig() to the global services
container. This is not meant to be used in practice, but there were
two places where this was missing: WebInstallerOutput, and various
PHPUnit tests.
* Add missing setConfig() to WebInstallerOutput.
* Add missing setConfig() to various tests. Most tests were already
doing this correctly and using the standard mock from
ResourceLoaderTestCase. Upon switching the last few tests as well,
I uncovered various errors due to the mock missing some settings
that the tested code uses, so these have been added now to
ResourceLoaderTestCase.
Bug: T277728
Change-Id: I44f16ec4e00423fb6f641e58fffc1d40e4571f01
`content` should be mapped to `content-thumbnails` when using list mode
rather than throwing a fatal.
Bug: T271441
Change-Id: Idb30a94803f1432e2c1f57d55eb8c8aef1368dfb
The value of content-media now depends on configuration.
These modules can be merged, as the expectation is both should
apply when the UseNewMediaStructure configuration flag is enabled.
For backwards compatibility map any content-thumbnail requests to
content-media
Bug: T278560
Change-Id: I10ba53b1ee217eae9e05182e989d56f5dc107722
Since both MediaWikiIntegrationTestCase and
MediaWikiUnitTestCase use MediaWikiCoversValidator,
subclasses do not need to do so, just tests that
extend the base PHPUnit test case directly
Change-Id: Ie8702625621d55130b04b6ef85114d3375a3b39c
This was added in 381499a675 in response to the issue that when a skin
adopts a feature that was new in MW 1.36, it caused an exception when
that version of the skin is then installed on MW 1.35. That's expected,
and generally helpful because the default support policy requires
like-for-like (MW 1.35 with REL1_35, this applies to all skins and
extensions, except for a few like Translate and SemanticMediaWiki).
However, while supporting LTS within master is unusual, there are
indeed the Translate/SemanticMediaWiki projects, and this problem has
already been solved years ago there by using a conditional with
$wgVersion. The proposed approach on T271441 actually also involves
such a conditional, but late at runtime instead of in the module
definition. Anyway, the way it was fixed in Timeless already solves
the problem by not passing down the invalid feature in the first
place, thus making the 381499a675 patch obsolete. I'm not aware of
any skin in Gerrit having such a compat policy, but if they exist,
they can fix it the same way, by defining the module in code with
a condition, just as Translate/SemanticMediaWiki do.
Bug: T271441
Change-Id: If97b3d04ac1e3355ee4f229db11e524d24ebd27a
* Remove odd use of ResourceLoaderFileModule::extractBasePaths()
SkinModule.php which was basically just returning $IP.
* Use getConfig()->get('ResourceBasePath') so that the test does
not need to override global configuration variables.
Follows-up 6845912bcf and f1457312dc.
Bug: T277728
Change-Id: I64262b48bb5dc71c53b4114eddb2f080a6a076f5
* Remove confusing dead code for T271441, as a continue statement at
the end of a loop body does not do anything. I'll reinstate the
exception once we've looked at whatever skin caused the issue and
have fixed those repos to not pass invalid options.
* Simplify list creation by avoiding the loops and duplicate logic.
Assign the list directly when possible, and otherwise store the
filtered keys.
* Remove unexpected access to `$this->getConfig()` in the constructor.
Follows-up a6c769e976, which worked by accident due to it defaulting
to global state. The injection from ResourceLoader::getModule happens
after the constructor. I am working on a patch to enforce this, and
thus this change needs to land first.
I've moved the logic to getStyleFiles() and boldly changed it to be
a simple skip rather than based on whether it was specified. If this
is a problem we can find a more complex way, but I wanted to try this
first. It seems there is no reason to load these, specified or not,
when the parser feature is not enabled. (Ref T266148)
The unit test was working around this violation in a similar way,
which is now redundant and removed in this patch.
* Document that compatibility method only works on map-form.
* Make applyFeaturesCompatibility() an internal and protected
method. I don't think we'd support extensions calling this
directly.
* Limit applyFeaturesCompatibility() to just dealing with
the features array instead of also handling other options
and applying of the default value.
Bug: T277728
Change-Id: I24a2b783570c888cedee66885647b3ed765e0132
1) Rename content-parser-output to `content-body`
Widen the scope of this module to apply to anything that can be rendered
as the article body. Improve the documentation on what is allowed here.
For early-adopter skins already using content-parser-output,
map the feature to the new name and add tests.
2) Reclaim the `mw-body-content` class for result of
SkinTemplate::wrapHTML
The `mw-body-content` has been used in skins to wrap various elements.
Going forward we will use it to wrap any HTML content generated by
OutputPage.
See dependent patches, which we're not directly depending on to avoid
a CI gremlin:
- I90d85c21f4a62e6697f24e3ce388445a0a53c2b0 (MonoBook)
- I11242e243c9a529b72972089af9ac2a8c906331a (Modern)
- I87942c60e62f6f14acdfeaa1836ace4eac9252ac (CologneBlue)
- I4c1b15d90bacbc9b13782a1d8f52e838ce8ecd83 (Vector)
Bug: T279388
Change-Id: I3a91b294fcb3724cd46743e497dff723de0490a6
It is not entirely meaningless. It might be an indicator that
the number of calls to a method is intentionally unlimited.
This is similar to e.g. an @inheritDoc PHPDoc comment that
marks a method as being "intentionally undocumented".
However, what's the meaning of being "intentionally
unconstrained"? Let's just not have any constraint then.
I feel all these ->expects( $this->any() ) bloat the test
code so much that it's never worth it.
Change-Id: I9925e7706bd03e1666f6eb0b284cb42b0dd3be23
After talking to Volker, the `elements` feature should not contain any
CSS classes. It should apply only on the tag level. I've updated documentation
to explain what's permitted here.
The link styling rules added in b24bff67ed are instead added to a new
feature `content-links`. The existing `content-links` is renamed to
`content-links-external` and for backwards compatibility anyone enabling
`content-links` without `content-links-external` will get the new styles.
The existing legacy feature for backwards compatibility loads the `content-links`
feature which addresses the problem in Timeless with plainlinks styling being lost.
Bug: T279693
Bug: T255717
Bug: T278576
Change-Id: I6e55ed2880782533ea61df98cf16dbf36483d895
It's the same and makes the test code much more readable, I
would like to argue.
Because of the was I split all the changes I made into smaller
patches this patch contains some other changes in the same
lines where I could not split them off. E.g. removal of
->any(), which is the default anyway and doesn't do anything.
Change-Id: Ib297b989d4aec33b31a4e33fe9d5032865b39be0
Ended up using
grep -Prl '\->setMethods\(' . | xargs sed -r -i 's/setMethods\(/onlyMethods\(/g'
special-casing setMethods( null ) -> onlyMethods( [] )
and then manual fix of failing test (from PS2 onwards).
Bug: T278010
Change-Id: I012dca7ae774bb430c1c44d50991ba0b633353f1
This is a reasonable default for all skins and is small enough
to not be a concern for existing skins.
Also: expand documentation so this is understood better.
Bug: T277218
Change-Id: Ia58037e2c32d76d5c7125772a3243b76ede45f3f
Modules that set "es6": true in their module definition will error when
a non-ES6 client tries to load them.
To detect ES6 support, this looks for native Promise support,
RegExp.prototype.flags, and non-BMP characters in variable names. All
browsers that lack full ES6 support fail at least one of those checks.
To flag modules as requiring ES6, this adds a ! to the end of their
version string. This takes up much less space than adding another
register() parameter (which would have to be at the end). It's hacky,
but we expect this feature to be relatively temporary, until we require
ES6 for running any JS at all (probably in about a year).
For distinguishing different types of errors thrown from
sortDependencies(), use e.name. We can't subclass Error properly because
that requires ES6.
Bug: T272104
Change-Id: I45670c910ff12eb422ae54c9fcf372e45c7b2bf1
The canonical way to enable debug mode is and will remain
through via 'debug=true'.
During the transition debug=2 will opt you in to the experimental
newer way. Anything that needs to be pinned to the old way for
compat can already start doing so by using debug=1 explicitly.
Once v2 is "ready", the default will flip and debug=1 will remain
for the foreseeable future to trigger the legacy behaviours.
Bug: T85805
Change-Id: Ieaf04e0c289646dd5d5b027b4f1f8278167b2d57
In the case of Monobook, Monobook styles were overriding a
normalize rule and failing to work.
Bug: T269618
Change-Id: Ifa346f60e3a50d874e68043dc4dece3fd8b41aa3
PHPUnit tests that are not based on MediaWikiIntegrationTestCase must
not access the global service container. This patch fixes two instances:
* GlobalIdGeneratorTest was tsting the getTimestampFromUUIDv1 via the
legacy static method in UIDGenerator. Changed to test the GlobalIdGenerator
method directly. Tests for the deprecated static methods should be
separate, if needed at all.
* DerivativeResourceLoaderContextTest does not reside under the "unit"
directory for strict unit tests, but was not based on
MediaWikiIntegrationTestCase, even though ResourceLoader logic heavily
relies on the global service container instance. Fixed by deriving the
test class from MediaWikiIntegrationTestCase.
Change-Id: I40b8364edc5bd88d60e62b0cd0fa51c6ae9d2298
The message cache is originally meant for mw.messages in JS,
which expects non-existent messages to be cleanly omitted.
There is minimal server-side and client-side handling in place for this.
For LESS, however, we were assuming that the blob is complete,
thus not feeding anything to the LESS compiler, thus leading to
a run-time failure where the LESS file can't be parsed at all
due to an undeclared variable.
This could happen sometimes during development or after upgrading
a wiki with a stale LocalisationCache that is still being updated
at that time.
Bug: T267785
Change-Id: I60ff4eb7dce1fee56470acc177afd29ee14b764f
* parent::setUp() should be first, and ::tearDown()
should be last
* Move tests that directly extend PHPUnit\Framework\TestCase
to /unit
Change-Id: I1172855c58f4f52a8f624e6d596ec43beb8c93ff
Previously, the order of styles outputted by this module was
arbitrary, based on the order of the feature files passed in.
This was not how it was intended to work - the ResourceLoaderSkinModule
should be aware of the correct ordering.
In the case of normalize for example - it should always be the first
file output.
The implementation is adapted to check for valid features once in
the constructor and to also consider a DEFAULT_FEATURES value has been
set.
We use the FEATURE_FILES constant to determine
the order of CSS output.
This is all documented in the class.
Bug: T269618
Change-Id: Iecbf11b5f09882e55d694651210d6d132d3cd412
Since the legacy parser will also be emitting the new media structure
that requires these styles. For now, the feature is only shipped by
default when $wgUseNewMediaStructure is enabled.
Bug: T51097
Bug: T266148
Change-Id: Id20d716ce145e0bae37621fd6e218a793b5332ae
Add the SkinLessImportPaths attribute for skin-specific LESS import
paths, which skins can use to override the mediawiki.skin.variables.less
file.
As a starting point, add the following 5 variables:
* device widths (3x)
To help phase out 'mediawiki.ui/variables'. These are
commonly used by MobileFrontend.
* @font-family-sans
Recommended by Volker. Used by multiple skins.
* @border-radius-base
Recommended by Volker as example of something that we currently
hardcode in MediaWiki core for Vector and OOUI/WikimediaUI
in 'mediawiki.widgets.datetime' but should instead be allowed
to vary by skin and OOUI theme.
Remove the hardcoded value for '@border-radius-base' in
various places in favour of importing from mediawiki.skin.
The default is a bare default of 0 (as border-radius is off
by default in the browser).
The value for Vector is restored there by I47da304667811.
The value for MonoBook is improved by I000f319ab31b.
Bug: T112747
Change-Id: Icf86c930a3b5524254bb549624737d3b9dccb032
Follow up to I755e5e6784481b419e35, which used array_unshift
to prepend the 'feature' stylesheets. This works as expected when
there is only one 'feature' enabled.
When there are multiple, use of unshift will effectivel reverse
the order as it unshifts then one at a time.
To mitigate this, collect them normally in the correct order,
and then prepend them all at once with array_merge.
Bug: T262507
Change-Id: Ibe2c9f8d024f6be06588a59df10a37681b60d6bc
For stylesheets provided by features such as normalize or elements,
skins will want to override some styles. Given that ResourceLoader
concatenates all styles, having feature styles override skin provided
styles is counter productive.
Bug: T261080
Change-Id: I755e5e6784481b419e35b338fe3a8129e94b9f61
* Move the method to SkinFactory and replaces usages.
* Inject $wgSkipSkins into the SkinFactory
Bug: T257993
Change-Id: I9869cf34c5e87cbad963f48db0649b3b7a252a4a
* Add a void return hint to methods that are not meant to return
anything. This helps catch accidental return statements in the
future, lets Phan better understand how methods are meant to be
used, and might also allow PHP to better optimise the compiled
code form (speculation).
I did not, however, add it to publicly extended methods as that
might mess with strict mode.
* Remove the internal getResourceLoader() method from MessageBlobStore.
This has been redundant since 3edaa0b37c.
The method was protected, and not considered stable to subclass
for extensions. Hence not a breaking change.
* Add Throwable typehint to formatException() and friends.
This is the narrowest one I could add given that the methods
called from here already enforce the same typehint.
Update the doc to match for now. There isn't a reason right now
to limit this only to Exception, and given this is the method
and not the catch statement itself, does not change behaviour.
* Remove unused ResourceLoader->getHookContainer().
Added within 1.35 cycle, safe to remove.
* Remove unexpected `@since` for `@internal` getHookRunner().
* Remove redundant `@internal` from ResourceLoaderMwUrlModule
methods, which itself is already `@internal`.
Change-Id: I68d33ff6feca7ef95282a7ff03eb9332adfde31c
The name change happened some time ago, and I think its
about time to start using the name name!
(Done with a find and replace)
My personal motivation for doing this is that I have started
trying out vscode as an IDE for mediawiki development, and
right now it doesn't appear to handle php aliases very well
or at all.
Change-Id: I412235d91ae26e4c1c6a62e0dbb7e7cf3c5ed4a6
This fixes an issue with HTML tags inside the <script> tag.
Remex also doesn't throw errors on attributes like @click, although it
does mangle them when producing DOM. To work around this, don't use DOM
serialization for the template HTML, but parse everything again using a
Remex parse+serialize pipeline that extracts the template and
(optionally) removes comments and strips whitespace.
One important effect of this change is that we'll have to forbid
self-closing tags in Vue templates, because Remex doesn't handle those
correctly (or rather, handles them *too* correctly). But on the up side,
we can now allow shorthands for v-bind/v-on/v-slot again.
Bug: T253334
Bug: T255587
Depends-On: I2253a2317187fe0d781ba5bfefab95e0f97d0a80
Change-Id: Id9a9728b7163601cc60bc587be07b70977d41970
ResourceLoader requires user scripts to be valid ES5.
Browsers have implemented most of the ES6 spec by now, with some
of the ES6 features actually having been around for a long time now.
As such, parse errors for otherwise valid ES6 scripts, from the POV of
the script author (which can include people editing their user scripts),
this error message can be inscrutable. Try to make it more explicit.
Change-Id: I20a6c705ac44c2c41b11e4a8f16675592b6d8a87
Some HTML parsers don't deal with this the way we want. libxml does, so
this test case passes, but I'm adding it for if we try switching to a
different HTML parser that breaks this.
Change-Id: I02d14e1d78320217646cda9691a78bcbf2005f52
