- The namespacing in Google's jsapi is very nice, with everything
being a member of a global "google" object. We would do well to emulate it, but migrating all JS to such a scheme is beyond the scope of the current project.
You somewhat contradict this approach by recommending against "class" abstraction below.. ie how will you cleanly load components and dependencies if not by a given name?
By module name. Each module can contain multiple files. I don't see any problem with allowing anonymous modules, as long as the caller is happy with the fact that such modules can't be used in dependencies or loaded on demand on the client side.
I agree we should move things into a global object ie: $j and all our components / features should extend that object. (like jquery plugins). That is the direction we are already going.
I think it would be better if jQuery was called window.jQuery and MediaWiki was called window.mw. Then we could share the jQuery instance with JS code that's not aware of MediaWiki, and we wouldn't need to worry about namespace conflicts between third-party jQuery plugins and MediaWiki.
Dependency loading is not really beyond the scope... we are already supporting that. If you check out the mv_jqueryBindings function in mv_embed.js ... here we have loader calls integrated into the jquery binding. This integrates loading the high level application interfaces into their interface call.
Your so-called dependency functions (e.g. doLoadDepMode) just seemed to be a batch load feature, there was no actual dependency handling. Every caller was required to list the dependencies for the classes it was loading.
The idea is to move more and more of the structure of the application into that system. so right now mwLoad is a global function but should be re-factored into the jquery space and be called via $j.load(); | |
That would work well until jQuery introduced its own script-loader plugin with the same name and some extension needed to use it.
[...]
We could add that convention directly into the script-loader function if desired so that on a per class level we include dependencies. Like mwLoad('ui.dialog') would know to load ui.core etc.
Yes, that is what real dependency handling would do.
- The "class" abstraction as implemented in JS2 has very little value
to PHP callers. It's just as easy to use filenames.
The idea with "class" abstraction is that you don't know what script set you have available at any given time. Maybe one script included ui.resizable and ui.move and now your script depends on ui.resizable and ui.move and ui.drag... your loader call will only include ui.drag (since the other are already defined).
I think you're missing the point. I'm saying it doesn't provide enough features. I want to add more, not take away some.
You can remove duplicates by filename.
[...]
We want to move away from php code dependencies for each javascript module. Javascript should just directly hit a single exposure point of the mediawiki api. If we have php code generating bits and pieces of javascript everywhere it quickly gets complicated, is difficult to maintain, much more resource intensive, and requires a whole new framework to work right.
Php's integration with the javascript should be minimal. php should supply configuration, and package in localized msgs.
I don't think it will be too complicated or resource intensive. JS generation in PHP is very flexible and you admit that there is a role for it. I don't think there's a problem with adding a few more features on the PHP side.
If necessary, we can split it back out to a non-MediaWiki standalone mode by generating some static JS.
What is your reason for saying this? Have you worked on some other framework where integration of PHP and JavaScript has caused problems?
- Central registration of all client-side resources in a global
variable would be onerous and should be avoided.
You can always add to the registered global. This works well by having the php read the javascript file directly to ascertain the global list. That way your javascript works stand alone as well as integrated with a script-loader that provides localization and configuration.
There's a significant CPU cost to loading and parsing JS files on every PHP request. I want to remove that behaviour. Instead, we can list client-side files in PHP. Then from the PHP list, we can generate static JS files in order to recover the standalone functionality.
[...]
That sounds cleaner than the present outputPage and Skin.php and associated script-loader grafting. Having a cleaner system would be nice... but will probably break skins and other stuff... or have OutputPage and Skin old api mappings or change almost every extension and break every 3rd party skin out there?
I think I'll probably break most third-party skins, if they have PHP code. We break them with just about every major release so there won't be much surprise there.
On this point, I think we need: * Easier management of non-PHP skins (i.e. CSS and images only) * Automated CSS generation (per original post) * Easier ways to modify the document structure, with less PHP involved. XSLT? * An interface in PHP that we can live with, so we don't feel obliged to keep breaking it.
I should be able to retain compatibility with non-skin extensions, and I won't break interfaces unnecessarily. But we're committed to an incremental development process, rather than a sequence of rewrites, and that means that some interfaces will get old and die within the 1.X.0 sequence.
[...]
- Like Wordpress, we could store minified and concatenated files in a
public cache and then link to that cache directly in the HTML.
That seems perfectly reasonable... Is the idea that this will help small sites that don't have things behind a squid proxy?
Yes, and it also benefits Wikimedia.
Although small sites seem to work oky with mediaWiki pages being served via php reading cached files.
Have you looked at the profiling? On the Wikimedia app servers, even the simplest MW request takes 23ms, and gen=js takes 46ms. A static file like wikibits.js takes around 0.5ms. And that's with APC. You say MW on small sites is OK, I think it's slow and resource-intensive.
That's not to say I'm sold on the idea of a static file cache, it brings its own problems, which I listed.
- The cache invalidation scheme is tricky, there's not really an ideal
system. A combination of cache-breaking parameters (like Michael's design) and short expiry times is probably the way to go. Using cache-breaking parameters alone doesn't work because there is referring HTML cached on both the server and client side, and regenerating that HTML periodically would be much more expensive than regenerating the scripts.
An option is to write out a bit of dynamic javascript to a single low expire static cached core script that sets the versions for everything that could be included. But that does not work well with live hacks. (hence the checking of filemodified date) ... If version updates are generally highly correlated with localization updates anyway... I don't see too much problem with old javascript persisting until a page is purged and rendered with the new interface.
I don't see benefit in hurting our cache rate to support ~new javascript~ with ~old html~
The performance impact of refreshing a common file once every hour or two is not large. Your code sets the expiry time to a year, and changes the urid parameter regularly, which sounds great until you accidentally cache some buggy JS into squid and you have no way to reconstruct the URID parameters and thus purge the object. Then you'd be stuck with the choice of either waiting a month for all the referring HTML to expire, or clearing the entire squid cache.
If there's a need for the versions of the HTML and JS to match, that should be handled rigorously, with old versions retained at the origin server, instead of relying on squid to keep a record of every object it's served.
[...]
- Unsafe construction of HTML. This is ubiquitous in the mwEmbed
directory and there will be a huge potential for XSS, as soon as user input is added. HTML construction with innerHTML can be replaced by document.createElement() or its jQuery equivalent.
I build a lot of html as static strings because its faster than generating every element with function calls. If you can inject arbitrary content into some javscript string then I imagine you can do so with the createElement as well. You don't gain much escaping already defined javascript. If you do something to get some value into some one elses JavaScript instance then you might as well call your evilJs directly. Perhaps I am understanding this wrong? Could you illustrate how that would be exploited in one case but not the other?
Say if MediaWiki emits an input box with a properly escaped attribute derived from user input
<input type="text" id="filename" value="<iframe src="http://example.com/"/>"/>
Then consider JS code such as:
dialog.innerHTML = "<div>" + document.getElementById( 'filename' ).value + "</div>";
This unescapes the value attribute, and puts the contents straight into HTML. The iframe will be created. This is a security vulnerability.
The alternative style used by jQuery UI is:
$j('<div/>') .text( $j('#filename')[0].value) ) .appendTo(dialog);
Or equivalently in plain DOM:
var div = document.createElement( 'div' ); div.appendChild( document.createTextNode( document.getElementById( 'filename' ).value ) ); dialog.appendChild( div );
The single text node contains the same literal text that was in the input box, no iframe element is created. You could think of it as implicit escaping, since if you ask for HTML back:
alert( dialog.innerHTML )
The browser will show you properly escaped HTML:
<div><iframe src"http://example.com/%22/%3E%3C/div%3E;
In OggHandler, I found that it was necessary to use innerHTML in some cases, because there were bugs involved with creating a Java applet and then changing its attributes. But I made sure that all the HTML I created was properly escaped, so that there was no possibility of arbitrary HTML being created, either from trusted or untrusted input.
It's best to escape even trusted input, for two reasons: * Correctness. Even trusted input can contain quotation marks. * Ease of review. Reviewers should not have to determine which of your inputs are trusted and which are untrusted in order to verify the safety of the code.
There's more on ease of review and other security issues in my article on the subject:
http://www.mediawiki.org/wiki/Security_for_developers
Security takes precedence over performance. There are better ways to improve performance than to open up your code to systematic exploit by malicious parties.
-- Tim Starling