Main issue for CAPTCHA's Drupal 8 migration.

Please submit pull-requests to https://github.com/chuva-inc/captcha/tree/8.x-1.x

CommentFileSizeAuthor
#70 CAPTCHA-Drupal_8_port-1949682-70.patch11.06 KBstr8
#69 CAPTCHA-Drupal_8_port-1949682-69.patch7.16 KBstr8
#1 CAPTCHA-Drupal_8_upgrade-1.patch2.3 KBdarrell_ulm
#4 CAPTCHA-Drupal_8_upgrade-1-do-not-test.patch2.3 KBdarrell_ulm
#6 CAPTCHA-Drupal_8_upgrade-2-do-not-test.patch50.62 KBelachlan
#7 CAPTCHA-Drupal_8_upgrade-3-do-not-test.patch50.96 KBelachlan
#8 CAPTCHA-Drupal_8_upgrade-4-do-not-test.patch99.33 KBelachlan
#9 CAPTCHA-Drupal_8_upgrade-5-do-not-test.patch99.34 KBelachlan
#14 CAPTCHA-Drupal_8_upgrade-5.patch99.34 KBwundo
#15 CAPTCHA-Drupal_8_upgrade-6-do-not-test.patch99.34 KBelachlan
#17 CAPTCHA-Drupal_8_upgrade-7-do-not-test.patch99.33 KBelachlan
#18 CAPTCHA-Drupal_8_upgrade-8-do-not-test.patch99.41 KBelachlan
#20 CAPTCHA-Drupal_8_upgrade-9-do-not-test.patch105.25 KBelachlan
#21 CAPTCHA-Drupal_8_upgrade-10-do-not-test.patch106.08 KBelachlan
#22 CAPTCHA-Drupal_8_upgrade-11-do-not-test.patch106.6 KBelachlan
#23 CAPTCHA-Drupal_8_upgrade-12-do-not-test.patch106.59 KBelachlan
#25 CAPTCHA-Drupal_8_upgrade-13-do-not-test.patch170.98 KBelachlan
#38 captcha-drupal8-port-config-api-1949682-38.patch6.59 KBm1r1k
#39 interdiff.txt2.26 KBm1r1k
#39 captcha-drupal8-port-config-library-apis-1949682-39.patch8.5 KBm1r1k
#40 interdiff.txt137.28 KBm1r1k
#40 captcha-drupal8-port-config-libraries-form-apis-psr4-1949682-40.patch143.22 KBm1r1k
#43 captcha-drupal8-port-1949682-43.patch282.13 KBm1r1k
#45 captcha-drupal8-port-1949682-45.patch286.45 KBm1r1k
#48 captcha-drupal8-port-1949682-47.patch287.59 KBm1r1k
#50 captcha-drupal8-port-1949682-50.patch277.18 KBm1r1k
#50 interdiff.txt25.71 KBm1r1k
#52 captcha-drupal8-port-1949682-52.patch278.37 KBm1r1k
#54 captcha-drupal8-port-1949682-57.patch278.63 KBm1r1k
#54 interdiff.txt1.66 KBm1r1k
#56 interdiff.txt8.9 KBjeqq
#56 captcha-drupal8-port-1949682-56.patch281.08 KBjeqq
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

darrell_ulm’s picture

Status: Active » Needs review
FileSize
2.3 KB

Attached is the patch to upgrade the module to Drupal 8.

Cheers,

Pass the following option to git commit to attribute authorship to this user:
--author="darrellulm <darrellulm@46284.no-reply.drupal.org>"

Status: Needs review » Needs work

The last submitted patch, CAPTCHA-Drupal_8_upgrade-1.patch, failed testing.

darrell_ulm’s picture

Status: Needs work » Needs review

The patch applies fine. This could be because there is no Drupal 8 branch to apply the scripts/run-tests.sh which is now at core/scripts/run-tests.sh.

Seems like it should run under 8.x-1.x test
ref: http://drupal.org/node/332678

Adding do-not-test flag.

Of course we're all learning the D8 stuff as the docs are worked out.

darrell_ulm’s picture

and the patch again renamed.

elachlan’s picture

Status: Needs review » Needs work

Here is a guide on converting a module.

https://drupal.org/node/1911346

I am working on it at the moment. It would be nice to have a 8.x dev branch to work against.

elachlan’s picture

All changes #4 made.

I have changed the info files on both captcha and image_captcha.

I also separated out the test files and placed them in the lib folder structure as per the guide.

I changed the .inc files and .module files to use t() instead of get_t.

The administration page seems to load but there is a problem with line 194 in captcha.inc.
$language = language(LANGUAGE_TYPE_INTERFACE);

This is in relation to this change: https://drupal.org/node/1450578

I have not made sense of it so far, I think it we have to use a service some how (see https://drupal.org/node/1539454).

Alternatively we could move towards using the controllers, which seems like a more segmented and proper approach.

elachlan’s picture

Made change for this issue.
https://drupal.org/node/1969794

Now examples work.

elachlan’s picture

Patch excluded some unversioned files..fixed.

elachlan’s picture

I talked to some people on IRC and through that I udpated the documentation with regards to https://drupal.org/node/1450578

I have changed the code accordingly.

It is now:
$language = Drupal::languageManager()->getLanguage();

elachlan’s picture

Title: Drupal 8 upgrade » Captcha Drupal 8 port

Changing title for better dashboard access.

wundo’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev

CAPTCHA 8.x-1.x branch created! ;)

wundo’s picture

Title: Captcha Drupal 8 port » CAPTCHA Drupal 8 port
wundo’s picture

Status: Needs work » Needs review
wundo’s picture

Re uploading the patch from #9 to trigger the testing.

elachlan’s picture

Status: Needs review » Needs work
FileSize
99.34 KB

Fixed missing warning image on admin/config/development/performance.

Currently the tests won't run, not sure on the cause. Believe its the branch failing testing.
See: https://qa.drupal.org/pifr/test/552048

elachlan’s picture

Currently getting this error:
Notice: Undefined index: id in theme_fieldset() (line 2870 of core\includes\form.inc).

To recreate:
enable captcha for:
user_login
user_login_block
user_pass
user_register_form

Then view the login page and click between the tabs.

elachlan’s picture

Final change for tonight.

Fixed some stuff to do with CaptchaBaseWebTestCase.

elachlan’s picture

More changes to tests.
646 passes, 114 fails, 104 exceptions, and 151 debug messages

elachlan’s picture

Because of Drupal 8 being HTML 5 I have removed the javascript added from this issue: https://drupal.org/node/881156

I am currently working to find a way to use something similar to the html details element, but constantly open. Currently If I set it to open, it doesn't work. I think it might be a bug in core.

Also firefox does not support html 5 as of yet.

elachlan’s picture

646 passes, 114 fails, 93 exceptions, and 151 debug messages

Getting better! Managed to get the captcha working on the user_login as it was renamed to user_login_form hence the changes to the install.

There is a hack for the details element at the moment, as the #open doesn't work on it. So I added it as an attribute.

elachlan’s picture

This should remove the exceptions. Will do a proper test tomorrow morning.

elachlan’s picture

710 passes, 14 fails, 0 exceptions, and 172 debug messages

Even better, but I am stuck with the testMultipleCaptchaProtectedFormsOnOnePage() test in CaptchaSessionReuseAttachTestCase.php

I believe allowCommentPostingForAnonymousVisitors() fails and causes the issues.

elachlan’s picture

705 passes, 0 fails, 0 exceptions, and 169 debug messages

Done for the core CAPTCHA module!!

Image CAPTCHA does not currently work, but shouldn't be as much of a hassle (famous last words).

elachlan’s picture

The structure of Image CAPTCHA kind of requires us to move to use the new forms system. So this is going to take a while.

Its worth the change though, because it brings all the different pages into form classes.

see: https://drupal.org/node/1800686 for more info on it.

I anticipated this and created an issue before #2024895: Move hook_menu() calls to routing system I will probably keep working here for now. Because CAPTCHA is already working, I will focus on image captcha. After both are working I will move CAPTCHA over to forms.

elachlan’s picture

705 passes, 0 fails, 0 exceptions, and 169 debug messages

Image CAPTCHA is done and moved to routing system for the administration. Both MENU_CALLBACKS will have to be migrated. I think to controllers instead of forms.

CAPTCHA has had the administration moved to routing system, except for the examples. I am unsure how /module/challenge is going to work.

Lots of the logic has been moved out of the admin.inc files.

This should be good to commit to the branch.

geerlingguy’s picture

A couple quick suggestions (to make D8 and CAPTCHA rock harder!):

  1. Could you put a #D8CX note/link on the project page (example: Honeypot), to give people confidence that CAPTCHA will be one of the awesome (and widely-used) modules they can take with them when they upgrade?
  2. Could an 8.x dev release be shown on the project page (assuming the module is now at a semi-working state...)? That would help non-devs be able to start testing/using CAPTCHA once D8 betas start rolling out—or even now, as I find the alphas are somewhat usable for testing/kicking the tires.

Many (if not most) of the people using Honeypot also have CAPTCHA installed, and I think it'd be awesome if both modules were ready to go on D8's launch day. It took a really long time for any good (and stable) spam prevention modules to be released for D7. Let's not make that mistake again!

elachlan’s picture

I'm not really the Dev on this. Just some guy doing my thing to help out. I agree with #D8CX, but that is a maintainers pledge. Not really something for us to decide on.

Does the patch work for you? I would love some feedback/patch reviews.

wundo’s picture

Looks like #23 adds some whitespace issues:

git apply CAPTCHA-Drupal_8_upgrade-12-do-not-test.patch 

CAPTCHA-Drupal_8_upgrade-12-do-not-test.patch:1508: trailing whitespace. 
CAPTCHA-Drupal_8_upgrade-12-do-not-test.patch:1515: trailing whitespace.
CAPTCHA-Drupal_8_upgrade-12-do-not-test.patch:1908: trailing whitespace.
CAPTCHA-Drupal_8_upgrade-12-do-not-test.patch:1931: trailing whitespace.
const CAPTCHA_WRONG_RESPONSE_ERROR_MESSAGE = 
CAPTCHA-Drupal_8_upgrade-12-do-not-test.patch:1939: trailing whitespace.
  
warning: squelched 6 whitespace errors
warning: 11 lines add whitespace errors.

Either way that one was applied to 8.x-1.x :)

wundo’s picture

The changes from #25 were also committed!

wundo’s picture

Btw, I've just committed a fix for the whitespaces on 8.x branch, it would be great if you reroll before writing the next patches (or --rebase your repository).

elachlan’s picture

Thanks Wundo!

#25 is the latest. It fixes a few issues.

Sorry about the whitespace!

I will rebase/re-write in the next 24hrs.

elachlan’s picture

Branch passed testing! :)

Next up will be moving more logic out of captcha.admin.inc into either controllers or forms.

Both of the last items from hook menu need to be moved and routes added:

  • captcha_point
  • examples

Same for image captcha:

  • image_captcha_font_preview

Then we can then see about moving the user logic to controllers.

Finally we will move to the configuration system. Which will neaten up the code a bit.

wundo’s picture

Added 8.x branch releases to the project home page!

elachlan’s picture

@Wundo - Could I please be a maintainer for 8.x branch?

wundo’s picture

Sure, that would be a honor ;)

wundo’s picture

Done!

elachlan’s picture

Thanks!! :)

m1r1k’s picture

Lets continue maintain 8.x branch :)
Apply new configuration API

m1r1k’s picture

m1r1k’s picture

Implement PSR-4, fix routing and forms api.

podarok’s picture

Status: Needs work » Needs review

test bot

Status: Needs review » Needs work
m1r1k’s picture

Status: Needs work » Needs review
FileSize
282.13 KB

Contains sniffer clean up, test fixes, image_captcha module port and lots of other stuff.

Status: Needs review » Needs work

The last submitted patch, 43: captcha-drupal8-port-1949682-43.patch, failed testing.

m1r1k’s picture

I've rewritten image generation controller using Symfony StreamedResponse class. Now image_captcha works nice and "pretty ready" to be used.

m1r1k’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 45: captcha-drupal8-port-1949682-45.patch, failed testing.

m1r1k’s picture

Status: Needs work » Needs review
FileSize
287.59 KB

Last fixes and diabling AdminTest because it needs full rewrite.

Status: Needs review » Needs work

The last submitted patch, 48: captcha-drupal8-port-1949682-47.patch, failed testing.

m1r1k’s picture

Status: Needs work » Needs review
Issue tags: +Drupalaton 2014
FileSize
277.18 KB
25.71 KB

Reroll

Status: Needs review » Needs work

The last submitted patch, 50: captcha-drupal8-port-1949682-50.patch, failed testing.

m1r1k’s picture

Status: Needs work » Needs review
FileSize
278.37 KB

Reverted minor changes in font binaries. Interdiff.txt is the same.

Status: Needs review » Needs work

The last submitted patch, 52: captcha-drupal8-port-1949682-52.patch, failed testing.

m1r1k’s picture

Status: Needs work » Needs review
FileSize
278.63 KB
1.66 KB

another reroll

Status: Needs review » Needs work

The last submitted patch, 54: captcha-drupal8-port-1949682-57.patch, failed testing.

jeqq’s picture

Add missing method and other fixes.

jeqq’s picture

Status: Needs work » Needs review

The last submitted patch, 56: captcha-drupal8-port-1949682-56.patch, failed testing.

m1r1k’s picture

Issue summary: View changes
Status: Needs review » Postponed

Lets move development to GitHub repo to make it easier and more flexible.
https://github.com/M1r1k/captcha

  • wundo committed 7d2f2bc on 8.x-1.x
    Issue #1949682 by jeqq, m1r1k, podarok, elachlan: Latest patch from the...
wundo’s picture

Issue summary: View changes
vladan.me’s picture

Latest changes are merged in github repo

elachlan’s picture

Linking related issue

EDIT:
Merged into drupal git. This should help fix #2353977: Fatal error on installation.

elachlan’s picture

Is it possible to get a dev release so that we can tag issues? It would make the meta game a little easier. It might also get more people involved.

wundo’s picture

There is already a dev release ;) 8.x-1.x-dev

m1r1k’s picture

Master branch on github has more the less workable version. Be aware of the image_captcha doesn't work with custom fonts (don't know why). You can check it on https://drupal.com/contact.

However I will merge plugin style rewrite soon that will have new image captcha generator lib, plugin system, per form settings and may be even test coverage :)

wundo’s picture

m1r1k if you're using github please send me a pull request ;)

tks

elachlan’s picture

Sorry Wundo! I posted it on the wrong issue!

The merged fixed an issue, so I just went ahead and did it.

str8’s picture

str8’s picture

FileSize
11.06 KB

hi, we have replaced old l() and url() functions with new methods. And created tab for image_captcha in module settings.

wundo’s picture

Thanks for the patch, I'd love to have a small summary from the changes you made... ;)

Berdir’s picture

Status: Postponed » Fixed

I'd suggest to close this issue. There's an 8.x-1.x branch, everything that has been worked on has been committed to that a long time ago.

The port is not complete, there are known issues, but we can track that with separate issues.

elachlan’s picture

Thanks Berdir!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.