These comments do not add anything. I argue they are worse than having
no comments, because I have to read them first to understand they
actually don't explain anything. Removing them makes room for actual
improvements in the future (if needed).
Change-Id: Iee70aad681b3385e9af282d5581c10addbb91ac4
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
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
Rather than only the 'private' group triggering embedding, allow modules
to explicitly specify if they should be embedded.
The default is still to only embed when the group is 'private', and the
'private' group is still special in that ResourceLoader::respond() will
still refuse to serve it from load.php.
Change-Id: Ib9a043c566822e278baecc15e87f9c5cebc2eb98
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
* Add fileName to cache key to fix T52919. The cached parsed error
message contains the filename, this should be part of the cache
key as otherwise two identical user scripts may report the same
error message, including " on line X of page Y" where Y is whichever
of the two pages first created the cache entry.
* Make the cache key global instead of per-wiki. There is no need
for this to be per-wiki.
Bug: T52919
Change-Id: I6c2718c53be7f6384a6486a4a8718ae7f423d216
* Simplify by using early return and getWithSetCallback.
* Add TTL (previously indefinite, now 1 week).
Bug: T52919
Change-Id: Ic95ba392cdb3bcc8081c77d2c2a3240548bed366
Some used a string value, others an array with 'message' property.
Standardise on the string value, which seems more intuitive.
Change-Id: I5caead7b7017d2bad660db02fb45a54a26bf3728
Follows-up 047b60b96d (ref T111481).
The if-condition compared the expanded paths, not the relative paths.
This meant there were two conditions under which the code will perform
a useless write that inserts *literally* the exact same JSON value.
1. The base directory ($IP) changes after a branch upgrade.
2. Paths contain '../', '//' or other unnormalized paths.
The latter caused various Echo and ULS methods to keep writing the
same value because one of their images is referenced in CSS using
'../'. When inserted in the database as relative path and then
expanded again at run-time and compared to the input value, they
don't match ("$IP/foo/../bar.png" != "$IP/bar.png") and cause a write.
Bug: T158813
Change-Id: I223c232d3a8c4337d09ecf7ec6e5cd7cf7effbff
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
Follows-up 1d15085bb3.
The column has a unique index for module name and skin/language pair.
Previously the write lock was on module name, which meant that
shortly after deployment, the following happens:
* Files change on disk.
* (1-5min pass)
* First startup module request after 5min http-cache expires. Detects
one or more changes and updates the version hash of that module.
* Web client subsequently requests this module (if used on that page).
The first time that request comes in, it's a varnish cache miss
and will make RL load all files from disk related to that module
and update the cache index in the module_deps table. At this point
most popular skin/lang pairs fail, except one. As a result, the
other rows remain stale.
* (7-30 days varnish expiry pass OR another change to the module deploys)
* Web client requests this module and tries to update its skin/lang pair
for that module.
One simple change in January 2016 changes jquery.tablesorter to load
a PNG file instead of a GIF file. Now, over a year later, there are
still a dozen skin/lang pairs in enwiki.module_deps with stale data,
which is causing various suble bugs, as well as filesystem calls for
files that don't exist.
Ref T113916 (refactor module_deps).
Ref T158105 (stale cache bug).
Bug: T158105
Change-Id: Ib6c024bfa8d35ce2d622ba4242291daedb507d5e
This should perform better and reduce internal lock contention on the
database server.
Bug: T158105
Change-Id: I1acfb0630946283b317cb929e8d7c3b2af757ecf
* The styles queue has always been top-only
(except for a few months in 2015).
* The top queue loads asynchronous since mid-2015. (T107399)
And LocalStorage eval, previously the last remaining non-async part
of module loading, is also async as of October 2016. (T142129)
* This change merges the bottom 'mw.loader.load()' queue with the top queue.
It also moves any other snippets potentially in the bottom queue still:
- embed: I couldn't find any private modules with position=bottom
(doesn't make sense due to their blocking nature). If any do exist,
(third-party extensions?), they'll now be embedded in the <head>.
- scripts: Any legacy 'only=scripts' requests will now initiate
from the <head>.
Bug: T109837
Change-Id: I6c21e3e47c23df33a04c42ce94bd4c1964599c7f
This is more consistent with LoadBalancer, modern, and inclusive
of master/master mysql, NDB cluster, and MariaDB galera cluster.
The old constant is an alias now.
Change-Id: I0b37299ecb439cc446ffbe8c341365d1eef45849
ResourceLoader modules can now carry a 'deprecated' option which can
be a boolean or an object with message key. This message or a default
deprecation message will be show whenever that module is used in production.
Note: This will not work in debug mode for ResourceLoaderFile modules
and this is deemed acceptable for the time being. We can revisit later.
Bug: T137772
Change-Id: Ib9ebd2d39a59fd41d8537e06884699f77b03580c
This allows dynamically loaded modules to depend on page-style modules
without it causing the page-style module to be loaded a second time.
* New method Module::getType() indicates whether a module is
a page-style module or supposed to be dynamically loaded.
* Emit warning from addModuleStyles() when given a module that is
not a page-style module (to be enforced later)
Bug: T92459
Bug: T87871
Change-Id: I8b6c6a10d965e73965f877c42e995d04202524f3
Follows-up da36f65433 which added this method with intent to use in
ResourceLoader::register(). However, the feature was redesigned to
not need this and the method was left behind.
Change-Id: I5ebc93805d0df6605bec94094bcd4eb2b70ff18d
* Fix errors spotted by new release
* Introduce "composer fix", which uses phpcbf to automatically fix some
errors spotted by phpcs.
* Drop $PHPCS_ARGS variable that didn't work on Windows, and add -s flag
* Remove rules from phpcs.xml that are now in MW-CS ruleset.
Change-Id: I13e2155695918c918b67497ac65b85a03897095e
MessageBlobStore class:
* Make logger aware.
* Log an error if json encoding fails.
* Stop using the DB table. WANObjectCache supports everything we need:
- Batch retrieval.
- Invalidate keys with wildcard selects or cascading check keys.
* Update tests slightly since the actual update now happens on-demand as
part of get() instead of within updateMessage().
ResourceLoader class:
* Remove all interaction with the msg_resource table. Remove db table later.
* Refactor code to use a hash of the blob instead of a timestamp.
Timestamps are unreliable and roll over too frequently for message blob store
because there is no authoritative source. The timestamps were inferred based on
when a change is observed. Message overrides from the local wiki have an
explicit update event when the page is edited. All other messages, such as
from MediaWiki core and extensions using LocalisationCache, have a single
timestamp for all messages which rolls over every time the cache is rebuilt.
A hash is deterministic, and won't cause needless invalidation (T102578).
* Remove redundant pre-fetching in makeModuleResponse().
This is already done by preloadModuleInfo() in respond().
* Don't bother storing and retreiving empty "{}" objects.
Instead, detect whether a module's message list is empty at runtime.
ResourceLoaderModule class:
* Make logger aware.
* Log if a module's message blob was not preloaded.
cleanupRemovedModules:
* Now that blobs have a TTL, there's no need to prune old entries.
Bug: T113092
Bug: T92357
Change-Id: Id8c26f41a82597e34013f95294cdc3971a4f52ae
This effectively reverts d6b4d3c537 and declines T97420.
This was previously attempted in b7c0e537eb.
Drop support for position "bottom" for addModuleStyles().
This feature was only recently introduced with the intent
to optimise page load performance, but had an adverse effect.
It increases chances of FOUC due to late discovery of these styles.
It also caused minor problems for some gadgets and extensions
that did not or were unable to set these flags. Some mobile
code was introduced around the same time as this feature and
was never given position=top.
Stylesheets that don't affect initial render or are only needed
on interaction should be loaded via addModules() instead; which
is handled by the asynchronous loader in JavaScript.
Change-Id: Ib9821b7b87cfc8a59062bb6ca358974fdb01ced1
Use BagOStuff::makeKey() and BagOStuff::makeGlobalKey() instead of
wfMemcKey() and wfGlobalCacheKey().
Change-Id: Id4e58c0e616b74de220faf13ba7c9ea606fef8c1
Using the following command line, I have found doc comments (and
a wfDeprecated() call) mentioning "1.26" when they should mention
"1.27" instead, which I have fixed manually:
git diff -M REL1_26 | grep --color=always -C 10 -iP \
'^\+.*\D1\.26(\D|$)' | aha > oldver.html
Follows-up these commits:
* 047b60b96d
* 169b7b98b5
* 25a44aa3e4
* 2fb2a3f14b
* 3f1e9fa268 [1]
* 8c84af70b6
* c0cb80beac
* d04a92a551
* d3b85592ea
[1] Release notes moved in I195dd1cf.
Because a release note stating that the UserRights hook is deprecated
was added in 37062a0c0d (before the branch cut), this commit does not
change the Hooks::run call that had been changed in 21206c8fbe.
Change-Id: I5a427f003e7e3b4559fe377bcdfdca466a570708
* Add support for a '/* @nomin */' annotation in ResourceLoader. If present in
JavaScript or CSS, the code will not be minified or cached. This allows
modules like the ResourceLoaderUserTokensModule to declare themselves unfit
for minification / caching without requiring a complicated refactor.
* Make ResourceLoader::filter() static, at the cost of not having minifier
errors in the ResourceLoader log bucket. (They will continue to be logged as
exceptions, however).
Change-Id: Ic1d802ee20565e61046bfbd8fd209bc56a4cbd6c
Re-use the md_skin database field for now as to not need a
schema change, beause this table is going away soon (T113916).
Bug: T113868
Change-Id: I7c7546ec58fd9be0447604989b908dd2084b0fe3
Make paths stored in the module_deps table relative to $IP. This ensures that when
the MediaWiki install path changes and/or if the location of the extension or skins
directory changes, that ResourceLoader's internal model is still accurate.
Previously when these paths change, ResourceLoader would exhibit various bugs.
1. Unable to detect changes in the module (if the directory no longer exists).
2. Point #1 is usually preceeded by one last cache invalidation as the content hash
of the file path changes (from a valid hash to an empty string).
3. Unnecessary cache invalidation (if both old and new directories exist). This
happens when a file is both an entry point (in the 'scripts' or 'styles' array)
and also a file dependency. At first they are de-duplicated by array_unique.
But after the disk path changes, the next check will result in the old path
being fetched from module_deps, and the new path from the live configuration.
This causes two changes that result in needless cache invalidation:
- The hash list contains one more item (T111481).
- The hash list contains both the old and new version of a file.
(or even alternate versions, e.g. when a change is backported to the old
wmf branch; it also affects wikis on the new branch due to the stale
file path still in the database).
It seems unusual to move a MediaWiki install, and usually we recommend third
parties to run update.php if site administrators do move their wiki. However
Wikimedia's deployment system implicitly moves the MediaWiki install continously
from e.g. "/srv/mediawiki/php-1.26wmf5" to "/srv/mediawiki/php-1.26wmf6".
This caused virtually all ResourceLoader caching layers to get invalidated every
week when another wmf-branch is deployed, thus breaking these file paths, which
changes the version hash, which then invalidates the cache.
Bug: T111481
Change-Id: I173a9820b3067c4a6598a4c8d77e239797d2659c
* Remove ResourceLoaderFileModule::getLessCompiler(). There is no reason for a
module to need to get a compiler in a different manner than
ResourceLoader::getLessCompiler().
* Add ResourceLoaderModule::getLessVars(). This method provides a means for
subclasses to easily inject custom LESS variables. The default implementation
simply returns an empty array.
* Make the $context parameter for ResourceLoaderFileModule::readStyleFiles()
non-optional (via graceful deprecation). The only callers I found were either
already calling it with a ResourceLoader context, or had a perfectly usable
ResourceLoaderContext in local scope.
* Make ResourceLoaderFileModule::{readStyleFile,getLessCompiler} require a
context. These methods are protected, so I didn't bother with a deprecation.
* Call ksort() on the LESS variables array in the only place it matters -- when
hashing its serialized representation to construct a cache lookup key. This
relieves getLessVars() subclasses from having to remember to re-sort the
variables array if they modify it.
* These changes make it possible to substantially simplify
ResourceLoaderEditToolbarModule, because the only thing it needs to do now is
implement its own getLessVars() method.
* This also allows it to be versioned like any other ResourceLoaderFileModule,
rather than having to use enableModuleContentVersion().
Change-Id: Ic3eab71691e502bfe19bdf4eb6f82cc679a7782f
