-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
btongminh@svn.wikimedia.org wrote:
+* (bug 11659) Urldecode image names in galleries
...
if ( strpos( $matches[0], '%' ) !== false )$matches[1] = urldecode( $matches[1] ); $tp = Title::newFromText( $matches[1] );
At this point it probably makes sense to go ahead and refactor this check & transformation into Title::newFromText itself -- it's already doing character reference decoding, so tossing in a URL decode doesn't sound too out of bounds, and it'll keep behavior consistent.
- -- brion vibber (brion @ wikimedia.org)
Brion Vibber schreef:
At this point it probably makes sense to go ahead and refactor this check & transformation into Title::newFromText itself -- it's already doing character reference decoding, so tossing in a URL decode doesn't sound too out of bounds, and it'll keep behavior consistent.
I have the feeling that that'll spawn some nasty bugs relating to pages with literal "%28" etc. in them.
Roan Kattouw (Catrope)
On Mon, Jun 2, 2008 at 8:34 PM, Roan Kattouw roan.kattouw@home.nl wrote:
Brion Vibber schreef:
At this point it probably makes sense to go ahead and refactor this check & transformation into Title::newFromText itself -- it's already doing character reference decoding, so tossing in a URL decode doesn't sound too out of bounds, and it'll keep behavior consistent.
I have the feeling that that'll spawn some nasty bugs relating to pages with literal "%28" etc. in them.
Roan Kattouw (Catrope)
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I'm not sure what you mean with this? If done properly it should not cause further breakage.
Bryan
Bryan Tong Minh schreef:
On Mon, Jun 2, 2008 at 8:34 PM, Roan Kattouw roan.kattouw@home.nl wrote:
Brion Vibber schreef:
At this point it probably makes sense to go ahead and refactor this check & transformation into Title::newFromText itself -- it's already doing character reference decoding, so tossing in a URL decode doesn't sound too out of bounds, and it'll keep behavior consistent.
I have the feeling that that'll spawn some nasty bugs relating to pages with literal "%28" etc. in them.
Roan Kattouw (Catrope)
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I'm not sure what you mean with this? If done properly it should not cause further breakage.
"If done properly" is the key sentence here. We'd have to check *every* call to Title::newFromText() (there are a lot of those), and the %28 corner case would probably still break in extensions who don't know about the new behavior. Anyway, changing the behavior of a central class such as Title is usually (usually) not a good idea.
Roan Kattouw (Catrope)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Roan Kattouw wrote:
Brion Vibber schreef:
At this point it probably makes sense to go ahead and refactor this check & transformation into Title::newFromText itself -- it's already doing character reference decoding, so tossing in a URL decode doesn't sound too out of bounds, and it'll keep behavior consistent.
I have the feeling that that'll spawn some nasty bugs relating to pages with literal "%28" etc. in them.
There are none, as that's been forbidden for a long time (since you can't round-trip them safely).
Literal % *is* allowed, but only when not followed by two hex digits.
- -- brion
Brion Vibber wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Roan Kattouw wrote:
Brion Vibber schreef:
At this point it probably makes sense to go ahead and refactor this check & transformation into Title::newFromText itself -- it's already doing character reference decoding, so tossing in a URL decode doesn't sound too out of bounds, and it'll keep behavior consistent.
I have the feeling that that'll spawn some nasty bugs relating to pages with literal "%28" etc. in them.
There are none, as that's been forbidden for a long time (since you can't round-trip them safely).
Literal % *is* allowed, but only when not followed by two hex digits.
Another corner case we need to consider is titles containing both "%" and "+", since urldecode() turns plus signs into spaces. Actually, the current behavior is arguably somewhat broken already (try typing the string "[[50% + 50% = 100%]]" into a wiki page and hit preview) but at least we should be careful not to break it any further.
Maybe the fix should be to only trigger the URL-decoding if the text matches /%[0-9A-fa-z]{2}/ rather than whenever it has a "%" sign in it.
wikitech-l@lists.wikimedia.org