Topic on Extension talk:OpenID

Jump to navigation Jump to search

Suggestion: change $wgOpenIDConsumerForce so that it fully specifies an OpenID provider (Url, logo, ...)

4
UnwashedMeme~mediawikiwiki (talkcontribs)

I have a patch (committed in my git repo) that I would like reviewed for inclusion. I clicked around and searched somewhat but didn't see any documentation here about submitting patches.

I've registered at gerrit.wikimedia.org; and I see that it gives me a git remote url customized to my user. Should I just push to there and that will launch a new review entry?

Cheers, Nathan Bird

This post was posted by UnwashedMeme~mediawikiwiki, but signed as UnwashedMeme.

Wikinaut (talkcontribs)
  • hi, what is that patch about ?
  • Is it based on my latest version Version 1.004 20120427 ?
  • have you tested locally everything so that you are fully sure your patch does not break anything ?
UnwashedMeme~mediawikiwiki (talkcontribs)

I'd left the question deliberately vague trying to create a generic "How do you submit patches" documentation bit.

I actually have a series of patches in git, pulled from gerrit as specified in the download section. The patches are currently based on 7e5b4d13b9 (master as of writing this).

This is about extending $wgOpenIDConsumerForce to be able to specify an OpenIDProvider instead of just a flat URL. This is useful if the provider varies by username and you wish to display the login form like the builtin providers.

  • If you specify $wgOpenIDConsumerForce as a string it continues to behave as before (tested).
  • If you don't specify $wgOpenIDConsumerForce it continues to behave as before (tested).
  • If you specify an OpenIDProvider, e.g. $wgOpenIDConsumerForce = new OpenIDProvider('wp', 'www.wordpress-site.com', 'Wordpress-site.com Username', 'http://www.wordpress-site.com/author/{username}/' ); it will display a login form asking for the username; skips rendering other providers' forms. (tested and using)

In the last case (or a future one with a specified list of providers, instead of just the one) the generic provider 'openid' (arbitrary url) may not be present. To handle this I removed the special case logic in

  • OpenIDProvider::getLoginFormHTML
  • skin/openid.js

The special case used to, for the provider 'openid', name the field 'openid_url' instead of "openid_provider_param_$id". There is now a hidden input 'openid_url' always present and the 'openid' provider is treated the same as everything else.


I tried to test the code paths that were effected by the change I made after each patch. There are quite a few options though so there is a chance that I missed one that would be a confounding factor. To ease review I tried to break it into several logically distinct patches that stepped in the right direction.

This post was posted by UnwashedMeme~mediawikiwiki, but signed as UnwashedMeme.

UnwashedMeme~mediawikiwiki (talkcontribs)

Searching for information on Gerrit I came across: http://www.mediawiki.org/wiki/Git/Tutorial#How_to_submit_a_patch; would this be a good procedure for this extension, which appears to be housed in the same domain?

Gerrit appears to prefer commits to not be a series; it looks like it creates separate reviews for each commit in a branch when you push. I've squashed some of the commits but I think it will be more palatable as several reviews unless you would specifically like to avoid that.

This post was posted by UnwashedMeme~mediawikiwiki, but signed as UnwashedMeme.

Reply to "Suggestion: change $wgOpenIDConsumerForce so that it fully specifies an OpenID provider (Url, logo, ...)"