[MediaWiki-CodeReview] [MediaWiki r87497]: New comment added

2011-06-07 Thread MediaWiki Mail
User Brion VIBBER posted a comment on MediaWiki.r87497.

Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/87497#c17743
Commit summary:

Per bug 28738 comment 4, pack ResourceLoader URLs by encoding 
foo.bar|foo.baz|bar.baz|bar.quux as foo.bar,baz|bar.baz,quux

* Expand these URLs in ResourceLoaderContext
* Build and emit these URLs in OutputPage::makeResourceLoaderLink() and in 
mw.loader
* Throw an exception in ResourceLoader::register() for module names that 
contain pipe characters or commas. Commas need to be forbidden for this packing 
feature to work. Pipes were already forbidden but weren't checked for

Comment:

phpunit test case added in r89666.

___
MediaWiki-CodeReview mailing list
mediawiki-coderev...@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview


[MediaWiki-CodeReview] [MediaWiki r87497]: New comment added, and revision status changed

2011-05-09 Thread MediaWiki Mail
User Mdale changed the status of MediaWiki.r87497.

Old Status: new
New Status: fixme

User Mdale also posted a comment on MediaWiki.r87497.

Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/87497#c16752
Commit summary:

Per bug 28738 comment 4, pack ResourceLoader URLs by encoding 
foo.bar|foo.baz|bar.baz|bar.quux as foo.bar,baz|bar.baz,quux

* Expand these URLs in ResourceLoaderContext
* Build and emit these URLs in OutputPage::makeResourceLoaderLink() and in 
mw.loader
* Throw an exception in ResourceLoader::register() for module names that 
contain pipe characters or commas. Commas need to be forbidden for this packing 
feature to work. Pipes were already forbidden but weren't checked for

Comment:

I think combination of buildModulesString and expandModuleNames is broken in 
the case of un-prefixed modules names. For example if you have a module name 
foo not mw.foo. 
in requesting foo|bar|cat, '''buildModulesString''' will give you: foo,bar|cat
And '''expandModuleNames''' will expand into
source lang=phpArray
(
[0] = .oo
[1] = .bar
[2] = cat
)
/source
Giving you {.oo:missing , .bar:missing } ... This was a little tricky 
to track down, since it missing module names don't throw an exception and fails 
silently in a loader script. ( maybe we should throw an exception when in debug 
mode for missing classes? ) 

If non-prefixed class names are not allowed, then an exception should be thrown 
if your module name is not prefixed by mw, jquery or ext prefix.


___
MediaWiki-CodeReview mailing list
mediawiki-coderev...@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview


[MediaWiki-CodeReview] [MediaWiki r87497]: New comment added

2011-05-09 Thread MediaWiki Mail
User Catrope posted a comment on MediaWiki.r87497.

Full URL: 
https://secure.wikimedia.org/wikipedia/mediawiki/wiki/Special:Code/MediaWiki/87497#c16753
Commit summary:

Per bug 28738 comment 4, pack ResourceLoader URLs by encoding 
foo.bar|foo.baz|bar.baz|bar.quux as foo.bar,baz|bar.baz,quux

* Expand these URLs in ResourceLoaderContext
* Build and emit these URLs in OutputPage::makeResourceLoaderLink() and in 
mw.loader
* Throw an exception in ResourceLoader::register() for module names that 
contain pipe characters or commas. Commas need to be forbidden for this packing 
feature to work. Pipes were already forbidden but weren't checked for

Comment:

Hmm, I thought I had my bases covered for prefix-less modules, guess not. Will 
look at this tomorrow.

___
MediaWiki-CodeReview mailing list
mediawiki-coderev...@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview