First of all, thanks for making this! I'm still trying to debug the "Invalid form POST data" issue I'm experiencing, but this module seems like the best approach.

With that said, I needed to clean up the module code and add a debug functionality to test things live. Patch incoming.

CommentFileSizeAuthor
#9 2682059-rebuild_ajax_forms-module-rewrite-3014335-9.patch9.7 KBfirewaller
#4 2682059-rebuild_ajax_forms-module-rewrite-3014335-4.patch9.7 KBfirewaller
#2 2682059-rebuild_ajax_forms-module-rewrite-3014335.patch9.53 KBfirewaller

Comments

firewaller created an issue. See original summary.

firewaller’s picture

firewaller commented
StatusFileSize
new 2682059-rebuild_ajax_forms-module-rewrite-3014335.patch9.53 KB

Patch attached.

firewaller’s picture

firewaller commented
Status: Active » Needs review
firewaller’s picture

firewaller commented
StatusFileSize
new 2682059-rebuild_ajax_forms-module-rewrite-3014335-4.patch9.7 KB

Updated patch with improved watchdogs

Neo13’s picture

Neo13 commented
Status: Needs review » Needs work

Patch fails to apply.

firewaller’s picture

firewaller commented

@Neo13 thanks for looking at this.

When I pull the latest changes via git and apply the patch with "git apply -v /[PATH]/2682059-rebuild_ajax_forms-module-rewrite-3014335-4.patch" or "patch -p1 < /[PATH]/2682059-rebuild_ajax_forms-module-rewrite-3014335-4.patch", the patch applies correctly in both instances.

Can you please confirm that you're using a cleanly pulled updated version of the module, the correct patch, and the appropriate patch command?

bmunslow’s picture

bmunslow commented

Hi @Neo13, I can confirm patch #4 applies correctly to latest version of the module.

@firewaller

Thanks for looking into this, your patches are very welcome.

I've looked into your patch, and it looks good!

You had a good idea to create a debugging flag that allows to turn it on or off more easily.

Just found one minor issue, the rebuild_ajax_forms_form function is defined twice: both in the rebuild_ajax_forms.admin.inc file and in the rebuild_ajax_forms.module file.

Also, the rebuild_ajax_forms.admin.inc file is never loaded, so we should settle on either way of loading the settings form, personally I think it should belong to the rebuild_ajax_forms.admin.inc file, who knows, we might need to expand on it one day.

Also, what do you think if we set the admin path of the settings form into the 'Development' area, since we are basically addressing developers who want to turn on/off debugging mode?

Could you change the settings path to admin/config/development/rebuild-ajax-forms ?

Thanks again for your work and interest on this module, looking forward to hearing your thoughts on this.

firewaller’s picture

firewaller commented

@bmunslow thanks for the reply and the feedback. I'll clean up based on your suggestions and probably have a new patch tomorrow.

firewaller’s picture

firewaller commented
Status: Needs work » Needs review
StatusFileSize
new 2682059-rebuild_ajax_forms-module-rewrite-3014335-9.patch9.7 KB

Attached is an updated patch with the above changes.

Neo13’s picture

Neo13 commented

I am sorry for the previous comment. The problem was with my IDE. I applied the patch and everything works OK.

By the way I have an issue when using this module (even before patching) that I get ajax error 200 on every page where there is an ajax form to replace. It is parseerror and empty result text. Do you have any idea what might be causing this?

firewaller’s picture

firewaller commented

@Neo13 no worries.

IRT your AJAX error, it is probably due to the callback you are using. I might recommend opening a new issue in the queue posting your code so we can work it out separately, I'd be happy to take a look.

Neo13’s picture

Neo13 commented

@firewaller Thanks. I figured it out already - Correctly return empty content for jQuery >= 1.9.

firewaller’s picture

firewaller commented
Status: Needs review » Reviewed & tested by the community
bmunslow’s picture

bmunslow commented
Title: Module rewrite » Module rewrite - add debugging flag and improve debug messages

  • bmunslow committed 1a633e7 on 7.x-1.x authored by firewaller 
    Issue #3014335 by firewaller: Module rewrite - add debugging flag and...
bmunslow’s picture

bmunslow commented
Status: Reviewed & tested by the community » Fixed

Changes commited, thanks for the work.

@firewaller, your latest patch didn't include any of the changes I mentioned, perhaps you uploaded the wrong patch... in any case, I just fixed the issues myself and also did a bit of refactoring.

Closing issue.

bmunslow’s picture

bmunslow commented
Status: Fixed » Closed (fixed)
firewaller’s picture

firewaller commented

@bmunslow whoops. I definitely made the changes, but must have posted the wrong patch. Thanks for sorting that out!