Create a single reCaptcha plugin for both versions. #5888

roland-d · 2015-01-26T08:30:03Z

This PR removes the noCaptcha plugin and merges it with the reCaptcha plugin. This way we have 1 reCaptcha plugin capable of using both the old and new keys depending on selected version.

Testing
To test, use reCaptcha version 1.0 with global keys and specific keys, all should continue to work as-is.
Also test version 2.0 with the new keys and you should get the new reCaptcha. The new keys also work with version 1.0.

After selecting the reCaptcha version, putting in the keys and selecting a theme. Check if the reCaptcha works on the front-end of the site.

peterlose · 2015-01-26T10:09:24Z

@test Works fine.

Old version:

New version:

peterlose · 2015-01-26T10:10:38Z

plugins/captcha/recaptcha/recaptcha.xml

+			showon="version:2.0"
+			filter="">
+			<option
+					value="light">PLG_RECAPTCHA_THEME_LIGHT</option>


Could you fix the spacing here? :)

infograf768 · 2015-01-26T10:14:06Z

Typo:
+PLG_RECAPTCHA_VERSION_DESC="Choose the reCaptcha version neede for your keys"

testing now.

brianteeman · 2015-01-26T10:28:32Z

administrator/language/en-GB/en-GB.plg_captcha_recaptcha.ini

@@ -7,16 +7,22 @@ PLG_CAPTCHA_RECAPTCHA_XML_DESCRIPTION="This CAPTCHA plugin uses the reCAPTCHA se
 PLG_CAPTCHA_RECAPTCHA="Captcha - ReCaptcha"


CAPTCHA - reCAPTCHA

As defined in style guide

infograf768 · 2015-01-26T10:31:08Z

@roland-d

I suggest to make it clearer that the new version (Version2 ?) does NOT accept Global keys.
Could be in the description or as a tip

brianteeman · 2015-01-26T10:32:42Z

I agree with with @infograf768

Perhaps
+PLG_RECAPTCHA_VERSION_1="1.0"
+PLG_RECAPTCHA_VERSION_1_DESC="Required if using Global CAPTCHA keys."
+PLG_RECAPTCHA_VERSION_2="2.0"
+PLG_RECAPTCHA_VERSION_2_DESC="Recommended version."

nikosdion · 2015-01-26T10:38:42Z

@test Success 👍

In detail:

Version 1.0 with global key: works as expected
Version 1.0 with site key: works as expected
Version 2.0 with global key: ReCAPTCHA complains about wrong key or domain as expected
Version 2.0 with site key: works as expected

Tested with front-end user registration and com_contact.

infograf768 · 2015-01-26T10:45:40Z

@test
Works fine here.

Just needs the improvements for the versions and we can merge, as we freeze langs tomorrow

ghost · 2015-01-26T11:22:43Z

Seems a bit weird that version 2.0 is the 'Recommended version.', but version 1.0 is the default active one.

roland-d · 2015-01-26T11:23:10Z

Thanks for testing everybody. I have updated the language files and believe they are good now.
@brianteeman I can't do a language string per version, so I have combined both into one.
Please check if all is OK now.

nikosdion · 2015-01-26T11:25:33Z

@nonumber Having read the past discussions, this strange default value seems to spring from the need to have a continuity for people using global keys. Version 2 API doesn't support global keys. If version 2 was the default, sites using global keys upgrading to Joomla! 3.4 would no longer have a functioning ReCAPTCHA misleading users into believing that Joomla! 3.4 is "broken". I'm not a fan of this default value either, but it's the only compromise solution to cater for the people who don't read the release notes, i.e. virtually everybody using Joomla!.

infograf768 · 2015-01-26T11:25:50Z

OK for me

@nonumber
I guess this is for B/C reasons

roland-d · 2015-01-26T11:27:13Z

Indeed that 1.0 is the default value, only reason is for backwards compatibility reasons. Those using global keys would on update have a broken reCAPTCHA. This way we ensure that keeps working.

wilsonge · 2015-01-26T11:30:29Z

administrator/language/en-GB/en-GB.plg_captcha_recaptcha.ini


 ; Params
+PLG_RECAPTCHA_VERSION_LABEL="Version"
+PLG_RECAPTCHA_VERSION_DESC="Version 1.0 is required if using Global CAPTCHA keys. Version 2.0 is the recommended version."


Being picky I think it's worth saying deprecated Global CAPTCHA keys to make it clear it's google not allowing these keys which is why we cannot accept them

Agreed but it MUST say reCAPTCHA not CAPTCHA
reCAPTCHA is the service from google. CAPTCHA is the generic name

brianteeman · 2015-01-26T11:34:04Z

@roland-d I would go with @wilsonge comment about stating that v1 is deprecated

roland-d · 2015-01-26T11:40:28Z

Updated the strings again :)

brianteeman · 2015-01-26T11:45:12Z

administrator/language/en-GB/en-GB.plg_captcha_recaptcha.ini


 ; Params
+PLG_RECAPTCHA_VERSION_LABEL="Version"
+PLG_RECAPTCHA_VERSION_DESC="Version 1.0 is required if using deprecated Global reCAPTCHA keys. Version 2.0 is the recommended version."


Sorry to be a PIA

Can you switch the order
+PLG_RECAPTCHA_VERSION_DESC="Version 2.0 is the recommended version. Version 1.0 is required if using the deprecated Global reCAPTCHA keys. "

Bakual · 2015-01-26T12:13:36Z

plugins/captcha/recaptcha/recaptcha.xml

@@ -36,12 +46,13 @@
 			size="50" />

 		<field
-			name="theme"
+			name="theme1"


You can't rename the existing parameter as it would break the plugin for updating users who selected a custom theme. Just leave that as theme and change the code to fit. The new param for v2 is fine with theme2.

Bakual · 2015-01-26T12:16:28Z

Can we show a deprecated warning in the module parameter view when v1 is selected or the version param is not yet saved? Could be done simple with a dummy formfield above the version field and the showon feature.
This way people would clearly see that they should choose v2 and it would solve the issue with the "wrong" default value (which we correctly have for BC reasons).

roland-d · 2015-01-26T12:31:18Z

@Bakual Thanks on the theme parameter. It's fixed now. As for showing the message, that is fine by me. Just need to know what the text needs to be :)

ghost · 2015-01-26T12:36:46Z

plugins/captcha/recaptcha/recaptcha.php

+			case '1.0':
+				$theme = $this->params->get('theme', 'clean');
+
+				$server = self::RECAPTCHA_API_SERVER;


No biggy, but to make it more in line with the code for v2:

$server = $app->isSSLConnection() ? self::RECAPTCHA_API_SERVER : self::RECAPTCHA_API_SECURE_SERVER;

Or even just remove these global vars and do:

$file = $app->isSSLConnection() ? 'https' : 'http'; $file .= '://www.google.com/recaptcha/api/js/recaptcha_ajax.js'; JHtml::_('script', $file);

roland-d · 2015-01-26T13:49:46Z

@nonumber Thanks for the suggestion, I have cleaned up the code per your suggestion.

ghost · 2015-01-26T13:53:18Z

Nice

Bakual · 2015-01-26T15:49:46Z

As for showing the message, that is fine by me. Just need to know what the text needs to be :)

Would need input from @brianteeman, but I would do something like "You have selected the deprecated version of this plugin. It's recommended to select version 2. Make sure you have supported keys for that version before switching."

brianteeman · 2015-01-26T17:35:33Z

Displaying a message is good but I think the tone and meaning of that proposed text is not right.

I think it needs to say something like
"You have selected Version 1. All new sites should be using Version 2. Version 1 is only maintained to provide support for Global Site keys which are no longer available from Google."

zero-24 · 2015-01-26T17:53:47Z

administrator/language/en-GB/en-GB.plg_captcha_recaptcha.ini


 ; Params
+PLG_RECAPTCHA_VERSION_1_WARNING_LABEL="You have selected Version 1. All new sites should be using Version 2. Version 1 is only maintained to provide support for Global Site keys which are no longer available from Google."


@roland-d @brianteeman I think we should use also here Version 1.0 and Version 2.0 or not?

Technically both would be wrong.
On 26 Jan 2015 17:53, "zero-24" notifications@github.com wrote:

In administrator/language/en-GB/en-GB.plg_captcha_recaptcha.ini
#5888 (comment):

; Params
+PLG_RECAPTCHA_VERSION_1_WARNING_LABEL="You have selected Version 1. All new sites should be using Version 2. Version 1 is only maintained to provide support for Global Site keys which are no longer available from Google."

@roland-d https://github.com/roland-d @brianteeman
https://github.com/brianteeman I think we should use also here Version
1.0 and Version 2.0 or not?

—
Reply to this email directly or view it on GitHub
https://github.com/joomla/joomla-cms/pull/5888/files#r23549545.

Oops my mistake the api is indeed 1.0 and 2.0 I had thought they were higher numbers

roland-d · 2015-01-26T19:43:59Z

Is it all good now or do we need more language updates?

brianteeman · 2015-01-26T20:55:32Z

All good to me - thanks

zero-24 · 2015-01-26T21:20:18Z

Thanks for testing and review moving to RTC based on various tests. Thanks!

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5888.}

infograf768 · 2015-01-27T07:20:22Z

Thanks. Merged after correcting also the installation sql files and en-GB.install.xml

mManishTrivedi · 2015-02-11T17:10:09Z

I want to work on it for Joomla but I am too late. 😔

Still I have submitted new plugin on JED for reCaptcha2. ☺️ Now I am finding new stuff for Joomla.

Create a single reCaptcha plugin for both versions.

555065a

jissues-bot added the PR-staging label Jan 26, 2015

roland-d added this to the Joomla! 3.4.0 milestone Jan 26, 2015

brianteeman mentioned this pull request Jan 26, 2015

CAPTCHA and reCAPTCHA (Legacy) #5595 #5664

Closed

peterlose reviewed Jan 26, 2015
View reviewed changes

Fixed a typo and spacing

d2fdbbb

brianteeman reviewed Jan 26, 2015
View reviewed changes

Unify reCAPTCHA/CAPTCHA spelling. Update version description.

9f218f4

wilsonge reviewed Jan 26, 2015
View reviewed changes

Fine-tuning the language strings

06d702c

brianteeman reviewed Jan 26, 2015
View reviewed changes

Updated version description.

e4d07e7

Bakual reviewed Jan 26, 2015
View reviewed changes

Keep the correct parameter name for theme

de1c7c7

ghost reviewed Jan 26, 2015
View reviewed changes

Clean up code for consistency.

67476a7

Add message when using version 1

d1fe3f2

zero-24 reviewed Jan 26, 2015
View reviewed changes

zero-24 mentioned this pull request Jan 26, 2015

Neues reCAPTCHA joomlagerman/joomla#87

Merged

Updated version numbering

fc3ed12

roland-d closed this in 43d726e Jan 27, 2015

roland-d deleted the recaptcha branch May 6, 2015 06:16

zero-24 mentioned this pull request Nov 7, 2015

J3.5 Beta1 #8304

Closed

infograf768 mentioned this pull request Jan 23, 2016

Change recaptcha default version to 2.0 #8967

Closed

		@@ -7,16 +7,22 @@ PLG_CAPTCHA_RECAPTCHA_XML_DESCRIPTION="This CAPTCHA plugin uses the reCAPTCHA se
		PLG_CAPTCHA_RECAPTCHA="Captcha - ReCaptcha"


		; Params
		PLG_RECAPTCHA_VERSION_1_WARNING_LABEL="You have selected Version 1. All new sites should be using Version 2. Version 1 is only maintained to provide support for Global Site keys which are no longer available from Google."

Uh oh!

Create a single reCaptcha plugin for both versions. #5888

Create a single reCaptcha plugin for both versions. #5888

Uh oh!

Conversation

roland-d commented Jan 26, 2015

Uh oh!

peterlose commented Jan 26, 2015

Old version:

New version:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infograf768 commented Jan 26, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infograf768 commented Jan 26, 2015

Uh oh!

brianteeman commented Jan 26, 2015

Uh oh!

nikosdion commented Jan 26, 2015

Uh oh!

infograf768 commented Jan 26, 2015

Uh oh!

ghost commented Jan 26, 2015

Uh oh!

roland-d commented Jan 26, 2015

Uh oh!

nikosdion commented Jan 26, 2015

Uh oh!

infograf768 commented Jan 26, 2015

Uh oh!

roland-d commented Jan 26, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brianteeman commented Jan 26, 2015

Uh oh!

roland-d commented Jan 26, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bakual commented Jan 26, 2015

Uh oh!

roland-d commented Jan 26, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roland-d commented Jan 26, 2015

Uh oh!

ghost commented Jan 26, 2015

Uh oh!

Bakual commented Jan 26, 2015

Uh oh!

brianteeman commented Jan 26, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roland-d commented Jan 26, 2015

Uh oh!

brianteeman commented Jan 26, 2015

Uh oh!

zero-24 commented Jan 26, 2015

Uh oh!

infograf768 commented Jan 27, 2015

Uh oh!

mManishTrivedi commented Feb 11, 2015

Uh oh!

Uh oh!