Project:Ubercart Payment Method Adjustments
Version:6.x-1.x-dev
Component:Code
Category:task
Priority:normal
Assigned:Unassigned
Status:active

Issue Summary

Code cleanup previously discussed here #760264-8: jQuery selector is not compatible with jquery_update

1) Implement drupal.behaviors + var context instead of document.ready (I know that in UC developers don't implement it, but they should, it is right way how to do it in Drupal - especially with jQuery 1.2.6, without $.live() support). Also adding uc-pma-processed class should be considered to avoid duplicate bindings.

2) Make URL in JS more flexible (support for i18n) - if you write URL in the way like base_path + 'some/url' it can't be modified by url_rewrite_inbound and outbound so it will not work on sites which uses modules which rewrites URL like 118n. The implementation of solution this is relatively easy, I suggest this solution to UC developers, year ago and they implement it in UC2.1 or 2.2 I believe.

3) Change name of internal functions in the module to respect of Drupal Standards. There are some special cases like dpm() or dvm() etc. from devel module, but I think that payment_method_adjustment_description() or _payment_method_adjustment() aren't privileged in any aspect...

4) Initialize all variables. (yes I'm pedantic ;-) )

Output from php-initialized tool

Uninitialized variable $items on line 84
Uninitialized variable $rows on line 118
Uninitialized variable $rows on line 127
Uninitialized variable $description on line 167
Uninitialized variable $ret on line 187

+ 5) Includes also my attempt to fix #760264: jQuery selector is not compatible with jquery_update.

AttachmentSize
uc_pma_cleanup.patch5.63 KB

Comments

#1

I had no luck with number 5.
Did anybody had success with the above patch?

ATM I'm looking at the original issue to see if I can make something out of it

#2

Hi bserem,

wojtha's patch includes changes for both the Javascript and module files. Depending our the tool or method you use to import these changes, it may or may not work as submitted. In my case, I use ECMerge's 'Import patch' feature which is great. All I needed to do was separate the .js and .module sections into separate files and apply.

BTW, thanks wojtha for taking the time to do and submit this cleanup.

--Sohodojo Jim--

#3

I have fixed the problem with this solution: http://drupal.org/node/760264#comment-3640982

I edited the js, of course the new js won't work without jquery_update, but it does the trick!
Thanks for the info though, I didn't know ECMerge (don't have any windows pc's for the moment) but I'll check it out!

#4

most of the uninitialized vars errors are cleaned up already, probably by Tomas while working on 7.x-1.x, but here's a remaining one and a couple of other minor cleanup items from 7.x-1.x

AttachmentSize
uc_pma-992630-uninitialized_vars_cleanup.patch 1.34 KB