GeezerVison is showing static-codereview.wikimedia.org — r87711 MediaWiki - Code Review archive
r87711 MediaWiki - Code Review archive
| Repository: | MediaWiki |
| Revision: | < r87710 | r87711 | r87712 > |
| Date: | 13:10, 9 May 2011 |
| Author: | catrope |
| Status: | reverted (Comments)
|
| Tags: | |
| Comment: | ( bug 28840) Commit patch by bawolff that encodes dots in ResourceLoader module names as exclamation marks in the generated URLs, so as to avoid triggering the recently added IE security measure and causing a blank 403 response. This broke RL in various ways for IE users.
This is by no means intended to be permanent, but it's the best way to unbreak RL for IE users while we work out how to fix this properly. |
| Modified paths: |
- /trunk/phase3/includes/resourceloader/ResourceLoader.php (modified) (history)
- /trunk/phase3/includes/resourceloader/ResourceLoaderContext.php (modified) (history)
- /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)
|
| Index: trunk/phase3/includes/resourceloader/ResourceLoader.php |
| — | — | @@ -708,7 +708,8 @@ |
| 709 | 709 | * Convert an array of module names to a packed query string. |
| 710 | 710 | * |
| 711 | 711 | * For example, array( 'foo.bar', 'foo.baz', 'bar.baz', 'bar.quux' ) |
| 712 | | - * becomes 'foo.bar,baz|bar.baz,quux' |
| | 712 | + * becomes 'foo!bar,baz|bar!baz,quux' |
| | 713 | + * The ! is for IE6 being stupid with extensions. |
| 713 | 714 | * @param $modules array of module names (strings) |
| 714 | 715 | * @return string Packed query string |
| 715 | 716 | */ |
| — | — | @@ -726,7 +727,8 @@ |
| 727 | 728 | $p = $prefix === '' ? '' : $prefix . '.'; |
| 728 | 729 | $arr[] = $p . implode( ',', $suffixes ); |
| 729 | 730 | } |
| 730 | | - return implode( '|', $arr ); |
| | 731 | + $str = implode( '|', $arr ); |
| | 732 | + return str_replace( ".", "!", $str ); # bug 28840 |
| 731 | 733 | } |
| 732 | 734 | |
| 733 | 735 | /** |
| Index: trunk/phase3/includes/resourceloader/ResourceLoaderContext.php |
| — | — | @@ -67,12 +67,13 @@ |
| 68 | 68 | /** |
| 69 | 69 | * Expand a string of the form jquery.foo,bar|jquery.ui.baz,quux to |
| 70 | 70 | * an array of module names like array( 'jquery.foo', 'jquery.bar', |
| 71 | | - * 'jquery.ui.baz', 'jquery.ui.quux' ) |
| | 71 | + * 'jquery.ui.baz', 'jquery.ui.quux' ) Also translating ! to . |
| 72 | 72 | * @param $modules String Packed module name list |
| 73 | 73 | * @return array of module names |
| 74 | 74 | */ |
| 75 | 75 | public static function expandModuleNames( $modules ) { |
| 76 | 76 | $retval = array(); |
| | 77 | + $modules = str_replace( "!", ".", $modules ); # bug 28840 - IE is stupid. |
| 77 | 78 | $exploded = explode( '|', $modules ); |
| 78 | 79 | foreach ( $exploded as $group ) { |
| 79 | 80 | if ( strpos( $group, ',' ) === false ) { |
| Index: trunk/phase3/resources/mediawiki/mediawiki.js |
| — | — | @@ -874,7 +874,7 @@ |
| 875 | 875 | var p = prefix === '' ? '' : prefix + '.'; |
| 876 | 876 | arr.push( p + moduleMap[prefix].join( ',' ) ); |
| 877 | 877 | } |
| 878 | | - return arr.join( '|' ); |
| | 878 | + return arr.join( '|' ).replace( /\./g, '!' ); |
| 879 | 879 | } |
| 880 | 880 | |
| 881 | 881 | |
Follow-up revisions
| Revision | Commit summary | Author | Date |
|---|
| r87712 | 1.17wmf1: Merge r87711 by hand. Because of recent changes in trunk, not a sin... | catrope | 13:20, 9 May 2011 |
| r88034 | Followup r87711: add ! to the forbidden characters list for module names, per CR | catrope | 09:24, 14 May 2011 |
| r88883 | (bug 28840) URLs with dots break because of IE6 security check... | catrope | 09:49, 26 May 2011 |
| r89397 | (bug 28840) If the query string hits bug 28235, redirect to a safer URL inste... | tstarling | 05:32, 3 June 2011 |
| r89627 | MFT r89397, r89558, etc.: bug 28840 IE URL extension | tstarling | 05:57, 7 June 2011 |
| r89628 | Merge r89627 from 1.18, equivalent to trunk r89558, r89397, etc.: bug 28840 I... | tstarling | 07:00, 7 June 2011 |
| r89676 | 1.17: MFT r82247, r87203, r87265, r87494, r87497, r87711, r87840, r88076, r89615 | catrope | 19:09, 7 June 2011 |
| r89727 | 1.17: Back out r87711 per CR on r89676 | catrope | 12:09, 8 June 2011 |
| r91440 | * Merged r89628 from 1.17: bug 28840 IE URL extension fixes.... | tstarling | 06:35, 5 July 2011 |
Comments
So, ! is a forbidden character now, which would join the list: comma, pipe, dot. Right ?
Should
Status & tagging log
- 09:50, 26 May 2011 Catrope (talk | contribs) changed the status of r87711 [removed: resolved added: reverted]
- 00:06, 20 May 2011 Krinkle (talk | contribs) changed the status of r87711 [removed: new added: resolved]
|