This was done as a "clever" shortcut to make sure tests a little
but shorter, but also made them less consistent with normal code.
Remove this in favour of 'class' or 'factory' options as needed.
Also remove a bunch of unneeded register() calls.
The tests cover everything affected by this change.
Side fix - isFileModule should reject modules with 'factory'
the same way it rejected raw objects and non-FileModule 'class'
cases already. This is now covered by tests as well.
Bug: T222637
Change-Id: I3996317dbcd780cc6e0f82c84e769c08a3fc42bb
Being a raw module means that when it is requested from load.php with
"only=scripts" set, then the output is *not* wrapped in an
'mw.loader.implement' closure *and* there no 'mw.loader.state()' appendix.
Instead, it is served "raw".
Before 2018, the modules 'mediawiki' and 'jquery' were raw modules.
They were needed before the client could define 'mw.loader.implement', and
could never be valid dependencies. Module 'mediawiki' merged to 'startup',
and 'jquery' became a regular module (T192623). Based on the architecture
of modules being deliverable bundles, it doesn't make sense for there to
ever be raw modules again. Anything that 'startup' needs should be bundled
with it. Anything else is a regular module.
On top of that, we never actually needed this feature because specifying
the 'only=scripts' and 'raw=1' parameters does the same thing.
The only special bit about marking modules (not requests) as "raw" was that
it allowed the client to forget to specify "raw=1" and the server would
automatically omit the 'mw.loader.state()' appendix based on whether the
module is marked as raw. As of Ie4564ec8e26ad53f2, the two remaining use
cases for raw responses now specify the 'raw=1' request parameter, and we
can get rid of the "raw module" feature and all the complexity around it.
== Startup module
In the startup module there was an interesting use of isRaw() that has
little to do with the above. The "ATTENTION" warning there applies to the
startup module only, not raw modules in general. This is now fixed by
explicitly checking for StartupModule.
Above that warning, it talked about saving bytes, which was an optimisation
given that "raw" modules don't communicate with mw.loader, they also don't
need to be registered there because even if mw.loader would try to load
them, the server would never inform mw.loader about the module having
arrived. There are now no longer any such modules.
Bug: T201483
Change-Id: I8839036e7b2b76919b6cd3aa42ccfde4d1247899
Remove use of a complete "namespace" for grouping and instead use dashes
within the first key segment for that.
This way, we can leverage the various features WANObjectCache provides
for statistics relating to a particular kind of thing being cached.
These statistics are currently combined for all of resourceloader,
which isn't useful. Mildly related to T223647.
Change-Id: I5df69e46ac436d04e765726a36fc3eb70697a7ed
lang=qqx and skin=fallback are the default values.
This change removes the default values from the load request of the
html5shiv module.
Before this change
<script src="/w/load.php?lang=qqx&modules=html5shiv&only=scripts&skin=fallback&sync=1"></script>
With this change
<script src="/w/load.php?modules=html5shiv&only=scripts&sync=1"></script>
Change-Id: Ie384ce0f7ab1bd0b6c2d3f0ca4a990c3cf3a7f15
The only remaining use of 'new ResourceLoader' is in tests, which have
been migrated in this commit to either passing the real config explicitly
(for integration tests), or by passing a HashConfig from a new
'getMinimalConfig' method which has only the keys required for the tests
to pass (e.g. avoid any ConfigExeption for unknown keys).
Also clean up some related code quality issues:
* Migrate wfScript() to $conf->get() so that the local Config is used,
instead of implicitly using global variables. This isn't deprecated for
MediaWiki generally, but done here to prepare ResourceLoader for becoming
a standalone library.
* Remove mocking of 'CacheEpoch' config, this is no longer used anywhere
in ResourceLoader.
* Change EmptyResourceLoader to use the minimal config by default and
remove code duplication by calling the parent.
Update the small number of uses that are integration tests, to explicitly
pass in the live config as needed. And for the one case that tests the
'startup' module, it no longer needs to register it manually given this
is part of ResourceLoader::__construct() by default.
Bug: T32956
Change-Id: I127346fd530fa66f205156e545758b1c29d0fac0
Use global variables directly as globals, except for the specific
case of accessing it when it may not exist. In those cases we
use the fact that undefined properties of an object yield the
`undefined` value which we can cast to false. Accessing an undefined
variable would yield a ReferenceError exception.
Change-Id: I1d9e9aa5845ba3c756ad6e31358d8594e003b04b
Also restore the order of registrations as it was before
last week with 47422fabe2. (That is, core modules are registered
before extension modules, in case of conflicts with a warning, the
core one wins).
Bug: T32956
Change-Id: I3a50508178159dfc8e5db1e218a5e6d10e2d4b2a
This has always been an odd case, as indicicated by the cross-class
comment references, and the fact that its test cases are already
in ResourceLoaderTest.php, for convenience, as that's also where
the creation of 'module name strings' is done and tested.
Actually move it there instead of pretending it is there.
Change-Id: Ied9569436cc78704a5c1b75eeebb73f8631350f6
This identified something we hadn't codified before which is that
we expect the 'startup' module to always exist. The code and tests
for ResourceLoaderClientHtml also assume this, which we now
acknowledge explicitly by leaving it in the constructor.
Bug: T32956
Change-Id: Iffc545eb32078aed0d059e5902a5c9382e14c641
Reduce our reliance on static state and configuration, and
propagate more state in explicit ways, through context, and
request parameters.
OutputPage creates ResourceLoaderContext and ResourceLoaderClientHtml
based on the configuration (via ResourceLoader::inDebugMode).
Everything within those classes should not need to check it
again.
* ResourceLoaderClientHtml:
Already doesn't check MW config, but it's test was still
mocking it. Removed now, and confirmed that it passes both
with true and false. The individual test cases set
debug=true/false as needed already.
It's sets were previously relying on the accidental behaviour
that within a unit test, we don't serialise over HTTP, which
meant that a pure PHP boolean would survive. With the new
raw `=== 'true'` check, this no longer works. Set it as a
string explicitly instead, which is the only thing we support
outside unit tests as well.
* ResourceLoaderContext:
Remove fallback to MW config when 'debug' is unset.
This is never unset in practice given that all load.php
urls have it set by OutputPage based on ResourceLoader::inDebugMode.
This change means that manually constructed ad-hoc load.php
urls that are missing 'debug=' parameter, will now always be
read as debug=false. This was the default already, but could
previously be changed through wgResourceLoaderDebug.
When changing wgResourceLoaderDebug, everything will still have
debug=true as before. The only change is when constructing load.php
urls manually and explicitly not set it.
Bug: T32956
Change-Id: Ie3424be46e2b8311968f3068ca08ba6a1139224a
It also removes some code duplication which is nice.
This unlocks various future changes, including:
* Making the `$config` parameter mandatory for the ResourceLoader class
constructor, which currently falls back to global state.
This should be deprecated and removed.
* Making it possible to instantiate the ResourceLoader class
without all the default MW modules being registered from
global state. E.g. move MW module registration from main class
constructor to ServiceWiring, and remove the 'EmptyResourceLoader'
class hack from unit tests, and use regular 'new ResourceLoader'
instead.
* Making ResourceLoader a standalone library (some day),
e.g. allowing it to be instantiated from a basic PHP script,
in a way that is still useful and perhaps able to serve
(most) RL modules without MW itself.
Bug: T32956
Change-Id: I4939f296c705b268e9cf8de635e923a739410470
Package files are files that are part of a module, but are not
immediately executed when the module executes. Instead, they are
lazy-excecuted when require() is called on them. Package files can be
scripts (JS) or data (JSON), and can be real files on the file system,
or virtual files generated by a callback.
Using virtual data files, server-side data and config variables can be
bundled with a module. Support for file-based require() allows us to
import npm modules into ResourceLoader more easily.
The require function passed to each script execution context, which was
previously a reference to the global mw.loader.require() function, is
changed to one that is scoped to the module and the file being executed.
This is needed to support relative paths: require( '../foo.js' ) can
mean a different file depending on the path of the calling file.
The results of require()ing each file (i.e. the value of module.exports
after executing it) are stored, and calling require() on the same file a
second time won't execute it again, but will return the stored value.
Miscellaneous changes:
- Add XmlJsCode::encodeObject(), which combines an associative array of
XmlJsCode objects into one larger XmlJsCode object. This is needed for
encoding the packageFiles parameter in mw.loader.implement() calls.
Bug: T133462
Change-Id: I78cc86e626de0720397718cd2bed8ed279579112
This was originally a global search and replace. I manually checked all
replacements and reverted them if (due to the lack of type hints) either
null (that would be 0 when counted) or a Countable object can end in the
variable or property in question.
Now this patch only touches places where I'm sure nothing can break.
For the sanity of the honorable reviewers this patch is exclusively touching
negated counts. You should not find a single `!== []` in this patch, that
would be a mistake.
Change-Id: I5eafd4d8fccdb53a668be8e6f25a566f9c3a0a95
Should be "string" not "String" and "array" not "Array" in
@param, @return and @var use cases. Also, minor typo fixes.
Change-Id: I9d5ebc5b741c6560907b95f7c0c4039da2861f4a
If something puts a string that's invalid UTF-8 in a JS config variable,
JSON serialization will fail on the entire config blob. Currently, this
causes the entire config blob to be silently dropped, which breaks all
JavaScript because elementary variables like wgPageName are missing.
This change makes this scenario fail loudly rather than quietly, by
throwing an exception. This also makes bugs like these easier to track
down.
Bug: T206475
Change-Id: Ief2ae00228389a23627d440dc1cd9a54cf2b6926
We already had a $filterCacheVersion variable, but it was
only used for the internal cache for JS and CSS minification,
which is not enough. If there is a breaking change in either
of these processes, we also need to invalidate version hashes.
This commit renames ResourceLoader::$filterCacheVersion to
ResourceLoader::CACHE_VERSION and takes it into account in
getVersionHash(). Adding it to getDefinitionSummary() is not
sufficient, because content-hashed modules also need to be
invalidated when there's a breaking change in the minifiers.
This cache version can also be incremented when there's a
breaking change in image embedding or LESS compilation,
although content hashing deals with that already, so we
could also add a separate cache version for those that's
only added to getDefinitionSummary().
Bug: T176884
Change-Id: Ife6efa71f310c90b9951afa02212b2cb6766e76d
* There is only a single non-test caller to this method in the entire
codebase, and no callers elsewhere (Wikimedia Git, Codesearch).
It's only used with an array, so remove the other unused code paths,
and mark it internal (to my knowledge, nothing ever used it outside
RL in the past, either).
* Add test coverage for the module indexing logic.
Change-Id: I9e0f95416d5b2fdd87323288231ee6d8c85d88e7
Follows-up dc3fc6cf81, which documented the reasoning for the
specific json flags in StartupModule. In wanting to re-use them
in a different module it became apparant that perhaps it was a
bit too conservative in only allowing the flags to be used in
a script HTTP response.
Lax the contract a little bit (that is, do more escaping) to also
allow safe re-use in HTML context.
I considered making these separate methods (e.g. forScriptResponse,
and forInlineScript) but decided not to because the vast majority
of callers would have to be forInlineScript eventhough the code
in question had no responsiblity or knowledge of it becoming an
inline script, because ResourceLoader allows most modules to
become embedded if they support a private or preview mode.
Those modes are implemented by calling makeModuleResponse, and
wrapping it an inline script. It seems appealing and simplifying
to the API to require that script output is always safe for
embedding rather than complicating the API for winning back
a literal handful of bytes in the edge case that a user-generated
string contains a '<' and was not embedded. I estimate that with
gzip, it will literally save only a single byte, even if used
multiple times. Let's focus optimisation efforts elsewhere :)
Change-Id: I7742dabba6750deecf6fbf51cf9a77ee8cbfc727
Added spaces around .
Removed empty return statement which are not required
Removed return after phpunit markTestIncomplete,
which is throwing to exit the test, no need for a return
Change-Id: I2c80b965ee52ba09949e70ea9e7adfc58a1d89ce
Follows-up a0809dfa5 (2015) which removed the feature for which
this would be used. No callers in MediaWiki core, Wikimedia Git,
or elsewhere indexed by Codesearch.
Bug: T65240
Change-Id: I56af8df3d072cd76a38595254812af6be268b134
The PHP interface still supports it without deprecation, but there
is no need for the client to support it given it is only ever called
from the startup module, which always passes it an object.
Follows-up 6815e35575, which did the same for `state(key, val)`.
Also simplify the implementation to avoid extra calls and optimise
for the only signature.
Mark the method as @private, and change the existing use of
@protected to @private (doesn't really make sense in JavaScript).
Change-Id: I24786f90bcfb256b2e7c37f7760bba1a5e399443
This significantly simplifies the getVersionHash implementation for
StartupModule, and fixes a couple of bugs.
Previously, the startup module's E-Tag was determined by the
'getDefinitionSummary' method, which combined the E-Tag values
from all registered modules, plus what we thought is all information
used by 'getScript' (config vars, embedded script files, list
of base modules, ...)
However, this were various things part of the manifest that it
forgot about, including:
* Changes to the list of dependencies of a module.
* Changes to the name of module.
* Changes to the cache group of module.
* Adding or removing a foreign module source (mw.loader.addSource).
These are all quite rare, and when they do change, they usually
also involve a change that *was* tracked already. But, sometimes
they don't and that's when bugs happened.
Instead of the tracking array of getDefinitionSummary, we now
use the 'enableModuleContentVersion' option for StartupModule,
which simply calls the actual getScript() method and hashes that.
Of note: When an exception happens with the version computation of
any individual module, we catch it, log it, and continue with the
rest. Previously, the first time such error was discovered at
run-time would be in the getCombinedVersion() call from
StartupModule::getAllModuleHashes(). That public getCombinedVersion()
method of ResourceLoader had the benefit of also outputting details
of that exception in the HTTP response output. In order to keep that
behaviour, I made outputErrorAndLog() public so that StartupModule
can call it directly now. This is covered by
ResourceLoaderTest::testMakeModuleResponseStartupError.
Bug: T201686
Change-Id: I8e8d3a2cd2ccd68d2d78e988bcdd0d77fbcbf1d4
* Remove use of MediaWiki's FormatJson class. Ref T32956.
* FormatJson::decode() was a direct alias of json_decode.
* FormatJson::encode() is a wrapper for json_encode that
sets `JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP`
by default, plus a boolean parameter to add JSON_PRETTY_PRINT.
Instead, use json_encode() directly. By default json_encode() escapes
slashes and non-ASCII unicode.
* Audit our uses of JSON encoding, document their needs, and set flags
as needed.
* Remove $mapToJson abstraction from ResourceLoaderStartUpModule.
* Always pretty-print the list of module names in the error message,
regardless of debug mode.
* Remove hacky indentation-adder from ResourceLoaderStartUpModule.
This was relying on there not being line breaks in the json output
outside debug mode, and adding a level of indentation to roughly
match the code in the JSON is substituted. This didn't match and
just generally doesn't seem worth it.
Change-Id: I3e09ddeb4d53c8980195c1855303c0ee25bd4c20
The use of global variables was deprecated in favour of
ResourceLoaderModule::getLessVars() on a per-module basis.
Also moved testLessFileCompilation case to the appropiate file as it
covers ResourceLoaderFileModule.php, not ResourceLoader.php.
Bug: T140804
Depends-On: Ib1b2808df2384473bfac47f53a5d25d7c9bbca2b
Depends-On: I96047f69d01c4736306df2719267e6347daf556f
Change-Id: If708087c85c80355c7e78f1768529b5f2e16ed07
This has no usage in non-test code anywhere in Wikimedia Git,
and was only used in the test suite for mw.loader in core,
and in the test suite for the NavigationTiming extension.
The core usage is as part of this commit. The usage in NavTiming
is updated by I240ced4e65988f9.
Bug: T127328
Depends-On: I240ced4e65988f96c7ece3772378c2c9a335fb9a
Change-Id: Ic17c797e528feaf07a4777709d705615fea353e5
This commit implements step 4 and step 5 of the plan outlined at T192623.
Before this task began, the typical JavaScript execution flow was:
* HTML triggers request for startup module (js req 1).
* Startup module contains registry, site config, and triggers
a request for the base modules (js req 2).
* After the base modules arrive (which define jQuery and mw.loader),
the startup module invokes a callback that processes RLQ,
which is what will request modules for this page (js req 3).
In past weeks, we have:
* Made mediawiki.js independent of jQuery.
* Spun off 'mediawiki.base' from mediawiki.js – for everything
that wasn't needed for defining `mw.loader`.
* Moved mediawiki.js from the base module request to being embedded
as part of startup.js.
The concept of dependencies is native to ResourceLoader, and thanks to the
use of closures in mw.loader.implement() responses, we can download any
number of interdependant modules in a single request (or parallel requests).
Then, when a response arrives, mw.loader takes care to pause or resume
execution as-needed. It is normal for ResourceLoader to batch several modules
together, including their dependencies.
As such, we can eliminate one of the two roundtrips required before a
page can request modules. Specifically, we can eliminate "js req 2" (above),
by making the two remaining base modules ("jquery" and "mediawiki.base") an
implied dependency for all other modules, which ResourceLoader will naturally
fetch and execute in the right order as part of the batch request.
Bug: T192623
Change-Id: I17cd13dffebd6ae476044d8d038dc3974a1fa176
Switch from passing a plain function to RLQ that calls depends
on using() from 'mediawiki.base' and 'jquery', to using the new
array format introduced in Ica7bb9c3bdb (T192623).
This ensures the code will keep after we change the fact that
RLQ is blocked by base modules.
Bug: T192623
Change-Id: Id50e6dcc28c13b021d8395a2da0d83b46f88e18c
Find: /isset\(\s*([^()]+?)\s*\)\s*\?\s*\1\s*:\s*/
Replace with: '\1 ?? '
(Everywhere except includes/PHPVersionCheck.php)
(Then, manually fix some line length and indentation issues)
Then manually reviewed the replacements for cases where confusing
operator precedence would result in incorrect results
(fixing those in I478db046a1cc162c6767003ce45c9b56270f3372).
Change-Id: I33b421c8cb11cdd4ce896488c9ff5313f03a38cf
The last remaining users of this feature were MobileFrontend and Minerva,
which have been migrated to direct imports.
Bug: T140807
Change-Id: I1a66a2ad314bde332297798520e5ec3e0e3d4c9b
This prevents cache churn when the wiki-global LESS variables vary
between wikis because the cache key is used as a "global" instead of
db-local. This is good for the common case, but should still explicitly
vary if the vars differ between wikis.
Bug: T191937
Change-Id: If12fd07a7062792205384150d6f5fd9a83f996cc
* Fix ResourceLoaderClientHtml to return what it was documented to
return, a WrappedStringList. It accidentally used the wrong join()
method, causing it to create a plain string too early.
* Update method documentations from ClientHtml::getBodyHtml to
BaseTemplate::getTrail for 'bottomscripts', and from
ResourceLoader::makeInlineScript for 'reporttime'.
* Update BaseTemplate::getTrail to join by new line instead of native string
concatenation.
This by itself would suffice for the most common case, but in order
to also account for the possibility of extensions using hooks for
'SkinAfterBottomScripts' that concatenate, update bottomScripts()
to pass a plain string to the hook and merge it later.
Change-Id: If0d227cb9db67229a27d489c082db790ea8e3840
Follows-up 70941efd35 which broke various public
signatures of the ClientHtml class that I'd prefer to handle
differently.
This commit mainly restores support for all previously public
signatures, and either removes the need for a parameter, or moves
it to the end of the original signature (as optional param).
* ClientHtml::getHeadHtml: Remove the positional/required parameter
that was added. Restoring the method to being a stateless computer
that requires no parameters. Pass the option via construct instead.
* ClientHtml::makeLoad:
- Make $nonce optional.
- Restore $extraQuery as optional.
* ResourceLoader::makeInlineScript: Document $nonce as optional
(matching the implementation).
Change-Id: Iaf33f2a060048e6606fba8d875b6d2953b21ef45
The primary goal here is a defense in depth measure to
stop an attacker who found a bug in the parser allowing
them to insert malicious attributes.
This wouldn't stop someone who could insert a full
script tag (since at current it can't distinguish between
malicious and legit user js). It also would not prevent
DOM-based or reflected XSS for anons, as the nonce value
is guessable for anons when receiving a response cached
by varnish. However, the limited protection of just stopping
stored XSS where the attacker only has control of attributes,
is still a big win in my opinion. (But it wouldn't prevent
someone who has that type of xss from abusing things like
data-ooui attribute).
This will likely break many gadgets. Its expected that any
sort of rollout on Wikimedia will be done very slowly, with
lots of testing and the report-only option to begin with.
This is behind feature flags that are off by default, so
merging this patch should not cause any change in default
behaviour.
This may break some extensions (The most obvious one
is charinsert (See fe648d41005), but will probably need
some testing in report-only mode to see if anything else breaks)
This uses the unsafe-eval option of CSP, in order to
support RL's local storage thingy. For better security,
we may want to remove some of the sillier uses of eval
(e.g. jquery.ui.datepicker.js).
For more info, see spec: https://www.w3.org/TR/CSP2/
Additionally see:
https://www.mediawiki.org/wiki/Requests_for_comment/Content-Security-Policy
Bug: T135963
Change-Id: I80f6f469ba4c0b608385483457df96ccb7429ae5
Deprecated since 1.30, only had one caller (MobileFrontend)
which has been fixed since.
Bug: T140804
Change-Id: I39b594e3082ebe321bae463780cfadbfb3de1d5c
* Move buildModulesString() call from doRequest() to batchRequest()
This keeps all module string "packing" logic located to the same function,
which is batchRequest(). It also means that the moduleMap object will not
leave the function, which helps in maintenance given it's very internal.
* Add comments to all the methods referring to each other.
* Explain why buildModulesString() is only a partial port, and the rest is
inlined in batchRequest().
* Minor changes to the JS and PHP implementation to better match each other.
- '$groups' -> '$moduleMap'.
- Remove redundant '$str'.
Bug: T188076
Change-Id: I7b0790606c456e492ab682faeb80f7e7fce7d9f8
It was deprecated to encourage callers to use makeInlineScript()
instead, because before makeInlineScript() was introduced, callers
would often manually wrap it in a <script> tag, which we don't want
because 1) We want de-duplication through WrappedString and 2)
callers sometimes forget to use Html.php methods that ensure
proper escaping as-needed.
However, there is still use in getting thet wrapped script without
also the wrapped HTML script tag. Namely, for use in http responses
that produce JavaScript, rather than HTML, where there cannot be
an HTML script tag.
There is at least one such caller (in SpecialJavaScriptTest) and
it's fine for there to be others.
Change-Id: Icfc285e12293c76dc68a220deb6816e32b058209
Follows-up b7eb243d92, which changed minification to be per-module,
but had as result that the little implement() wrapper is not always
minified, e.g. when outputting modules that have minification disabled
(such as 'user.tokens' and 'user.options').
Rather than introducing some complex minification cache for the
wrapper (with a find and replace for the variable content), simply
add a hardcoded minified version of the wrapper.
Change-Id: Iccf0d3408beab4387031cc55689394ff67e1e64b
I can see that "parent::__construct" literally calls the parent
constructor. I can see that stuff preceeded by the keyword "protected"
is protected. I really (really) don't need comments explaining such.
Change-Id: I7458e714976a6acd3ba6a7c93fdc27d03903df83
We currently have several counts and timings of individual pieces
of ResourceLoader backend logic (minification, module building, ..)
but no measure of the response overall.
This responseTime metric will effectively provide both a timing
measure as well as a backend request count.
Bug: T178350
Change-Id: I625a5eb90f5a4ea90aebf9292dfda0f1c5ae4f2e
For some varargs a variable name is added with suffix ,... as seen for
many other varargs
Some @param are swapped, because there are in the wrong order
Enable Sniff MediaWiki.Commenting.FunctionComment.ParamNameNoMatch
Change-Id: I60fec6025bce824d5c67563ab7b65ad6cd628ad8
ResourceLoaderModule objects gain a new method getPreloadLinks() which
returns an array with the meta data required to build a Link rel=preload
header according to the current draft for W3C Preload.
<https://w3c.github.io/preload/>
Another implementation of this is already in use in OutputPage for
preloading the logo image.
This array is formatted by the ResourceLoaderModule::getHeaders method,
which is implemented as "final" at this time, thus restricting use to
the Link rel=preload header.
Headers are exposed and process-cached, like all other content
(scripts, styles, etc.), through ResourceLoaderModule::getModuleContent,
and aggregated by ResoureLoader::makeModuleResponse.
I had hoped for the getPreloadLinks to be stateless (not vary on $context).
Whether something should be preloaded and what, should not vary on the
skin or language. However, while that conceptually holds true, the exact
url for any given resource may still vary. Even the main use case for this
feature (T164299, preloading base modules request) require $context to pass
down skin and lang to the load.php url.
Add full test coverage and example documentation.
Bug: T164299
Change-Id: I2bfe0796ceaa0c82579c501f5b10e931f2175681
And auto-fix all errors.
The `<exclude-pattern>` stanzas are now included in the default ruleset
and don't need to be repeated.
Change-Id: I928af549dc88ac2c6cb82058f64c7c7f3111598a
This fixes two bugs:
* 1) When two modules are requested, and the first one ends with ";"
inside a comment, the second module might become part of
that comment.
* 2) A request with script=only where the requested module content
ends in a statement without ";", but has a comment after it
that does ends with a semicolon, then in debug=false, mw.loader.state()
would be appended directly after the semicolon-less statement because
the check is performed before minification.
Previously:
script> foo()
script> // bar();
states> mw.loader.state( {} );
Became (minified separately):
script> foo()
states> mw.loader.state({});
Became (concatenated)
> foo()mw.loader.state();
Which is invalid code.
Both of these are now fixed.
Bug: T162719
Change-Id: Ic8114c46ce232f5869400eaa40d3027003550533
All ResourceLoaderFileModule subclasses are now required to support
the 'skinStyles' property in their module definition, and ideally
make use of it (but at least they must not fail violently).
Follow-up to 51eede0283.
Bug: T167478
Bug: T168088
Change-Id: I35a12a451bf2695818702df1bbd1708173a3f9ce
In c8ad83310f, 'oojs-ui-core.styles'
was changed to use ResourceLoaderOOUIFileModule instead of plain
ResourceLoaderFileModule. This had the unintended consequence that
ResourceLoader::isFileModule() no longer returned true for it, and
this $wgResourceModuleSkinStyles no longer worked, breaking styling
in Vector.
Additionally, the new ResourceLoaderOOUIFileModule did not respect
the skinScripts/skinStyles options passed in the module definition
(therefore also those from $wgResourceModuleSkinStyles). Merging
them turns out to be a major pain, but it can be done.
Bug: T167042
Change-Id: I7547bbe996467745c1f0b168b40f27eb95c6238d
includes/resourceloader/ResourceLoaderOOUIModule.php
* New trait centralizing some logic for dealing with OOjs UI themes,
previously duplicated in OutputPage, ResourcesOOUI.php and
ResourceLoaderOOUIImageModule.
* Follow-up change I74362f0fc215b26f1f104ce7bdbbac1e106736ad uses this
as a base to allow skins/extensions to define new OOjs UI themes.
resources/Resources.php
resources/ResourcesOOUI.php
includes/resourceloader/ResourceLoader.php
* OOjs UI resource module definitions are moved back to their rightly
place in Resources.php. They are again (almost) normal and static.
* Theme-specific logic is now handled by the module code, definitions
only specify 'themeScripts'/'themeStyles'/'themeImages'.
* ResourcesOOUI.php is deleted and no longer loaded by ResourceLoader.
includes/resourceloader/ResourceLoaderOOUIFileModule.php
includes/resourceloader/ResourceLoaderOOUIImageModule.php
* Glue code previously existing in ResourcesOOUI.php now lives here.
* Use the ResourceLoaderOOUIModule trait to avoid code duplication.
Change-Id: I39cc2a735d9625c87bf4ede6f5fb0ec441d47dcc
This should work the same way as registering API modules via a factory callback.
Point in case: Ifb8611473a971 could avoid global state using this mechanism.
Change-Id: Ifbf29006141ce2a2dff42efa352f406502a06bc6
Install the backtrace collector very early, so that we can get the
backtrace even if headers were sent from LocalSettings.php.
Bug: T157392
Change-Id: I9bc732b34481c95afb5362e135a87bd4302498e2
It's unreasonable to expect newbies to know that "bug 12345" means "Task T14345"
except where it doesn't, so let's just standardise on the real numbers.
Change-Id: I6f59febaf8fc96e80f8cfc11f4356283f461142a
Use of &$this doesn't work in PHP 7.1. For callbacks to methods like
array_map() it's completely unnecessary, while for hooks we still need
to pass a reference and so we need to copy $this into a local variable.
Bug: T153505
Change-Id: I8bbb26e248cd6f213fd0e7460d6d6935a3f9e468
Make ResourceLoader error formatting the same as everywhere else.
Which means if wgShowExceptionDetails is enabled locally, the
trace will be included as well.
This matches logic in MWExceptionRenderer.
Also move the typical error handling used by respond() to a
utility method to reduce duplication of code and avoid mistakes.
Change-Id: If04ae99618e4a758ed0f9dd2b555496b76da29de
When getScript (or some other method used in a module response)
throws an error, only that module fails (by outputting mw.loader.state
instead of mw.loader.implement). Other modules will work.
This has always been the case and is working fine. For example,
"load.php?modules=foo|bar", where 'foo' throws, will return:
```js
/* exception message: .. */
mw.loader.implement('bar', ..)
mw.loader.state('foo', 'error')
```
The problem, however, is that during the generation of the startup
module, we iterate over all other modules. In 2011, the
getVersionHash method (then: getModifiedTime) was fairly simple
and unlikely to throw errors.
Nowadays, some modules use enableModuleContentVersion which will
involve the same code path as for regular module responses.
The try/catch in ResourceLoader::makeModuleResponse() suffices
for the case of loading modules other than startup. But when
loading the startup module, and an exception happens in getVersionHash,
then the entire startup response is replaced with an exception comment.
Example case:
* A file not existing for a FileModule subclass that uses
enableModuleContentVersion.
* A database error from a data module, like CiteDataModule or
CNChoiceData.
Changes:
* Ensure E-Tag is still useful while an error happens in production
because we respond with 200 OK and one error isn't the same as
another.
Fixed by try/catch in getCombinedVersion.
* Ensure start manifest isn't disrupted by one broken module.
Fixed by try/catch in StartupModule::getModuleRegistrations().
Tests:
* testMakeModuleResponseError: The case that already worked fined.
* testMakeModuleResponseStartupError: The case fixed in this commit.
* testGetCombinedVersion: The case fixed in this commit for E-Tag.
Bug: T152266
Change-Id: Ice4ede5ea594bf3fa591134bc9382bd9c24e2f39
This is a pure documentation change. It mostly removes empty lines from
comments (and entirely empty comments), as well as adds a few missing
documentation blocks and fixes a minor mistake. I hope it's ok to have
this in one patch. I can split it, please tell me.
Change-Id: I9668338602ac77b903ab6b02ff56bd52743c37c4
This change resulted in unreasonable feature loss (human-readable
limit report was gone). Three months and multiple followups later,
the functionality is still not completely restored. Given lack
of response from the original author, I think it is time to revert
and reconsider, especially since the 1.28 release is soon.
A machine-readable limit report would be a very useful feature,
but not at the cost of losing human-readable limit report.
This reverts the following commits:
* Move NewPP limit report HTML comments to JS variables
b7c4c8717f
* Only pretty-print the parser report JS vars
28adc4d7ee
* Show wgPageParseReport on page previews too
1255654ed5
* Re-add human readable parser limit report
0051f108b9
* Restore hooks.txt for ParserLimitReportFormat
4663e7a737
Resolved minor merge conflicts in OutputPage (with 80e5b160)
and release notes.
Bug: T110763
Bug: T142210
Change-Id: Id88c8066fae3f369e8977b4b7488f67071bdeeb7
Use HTTPS instead of HTTP where the HTTP link is a redirect to the HTTPS link.
Also update some defect links.
Change-Id: Ic3a5eac910d098ed5c2a21e9f47c9b6ee06b2643
It looks like there is something missing after the last statement
Also remove some other empty lines at begin of functions, ifs or loops
while at these files
Change-Id: Ib00b5cfd31ca4dcd0c32ce33754d3c80bae70641
Follows-up 6fa1e56. This is already fixed for http caches by
shortening the Cache-Control max-age in case of a version mismatch.
However the client still cached it blindly in mw.loader.store.
Resolve this by communicating to the client what version of the module
was exported. The client can then compare this version to the version
it originally requested and decide not to cache it.
Adopt the module key format (name@version) from mw.loader.store
in mw.loader.implement() as well.
Bug: T117587
Change-Id: I1a7c44d0222893afefac20bef507bdd1a1a87ecd
* Keep this out of makeLoaderImplementScript() to keep it more generic
and to simplify future refactoring.
* Remove now-broken test case that asserted that the output varies
by global debug mode.
* Make the test responsible for wrapping in XmlJsCode. Previously
this magically happened because the module name was "user" in the
last test case.
* Make makeLoaderImplementScript protected. It's not used anywhere
outside ResourceLoader and we should keep it that way.
Test plan:
* Verify output unchanged:
- load.php?modules=user&only=scripts&user=Admin (raw code)
- load.php?modules=user&user=Admin (implement with unwrapped string)
- load.php?modules=jquery.client (implement with closure)
Change-Id: I527d01926fb6e4ce68c931695d830cdb9ceb608c
