D6 backport

Thanks for the patch, but it doesn't apply. I think you might have had your git refs mixed up when you did your diff?

Nope. Never mind. That was my bad. I was applying it to the d7 branch. Derp.

Although I managed to get your patch applied to the 6.x branch... I don't understand what the patch is supposed to do. It totally breaks functionality. There are function calls in there to functions that weren't introduced until D7 (e.g. path_is_admin(), ile_create_url()). Additionally the AHAH javascript was removed and replaced with the "AJAX" functionality which was also not introduced until D7. It basically looks like you just did a diff between the drupal 7 and drupal 6 versions of bg_image.module ... Is that correct?

Hi

Ah, I see. Sorry, I didn't mean to jump to any conclusions there. At first glance it just looked like there were a lot of D7isms left behind so I thought maybe you'd diff'd the wrong refs or something in git. My mistake!

Okay so, the only major note I have is regarding the ajax functionality. Are you using the AJAX Module by any chance? The #ajax form property was not introduced until drupal 7. In drupal 6 I believe it's the ajax module that brings that functionality to the table. I would prefer to go back to the core #ahah functionality rather than introduce a module dependency...

Regarding the path_is_admin function, it was actually there before, but the patch removes it here:

 /**
  * Returns an options list of css repeat options
@@ -375,14 +573,34 @@ function bg_image_css_repeat_options() {
   );
 }
 
+/***********************************************************************
+ * HELPER FUNCTIONS - Private
+ ***********************************************************************/
+
 /**
- * Returns true if the current page is an 'admin' page
+ * Converts a path to a uri relative to the public files directory
+ *
+ * @param $path
+ *    The path to convert. Can be an absolute or relative path. If the path
+ *    contains the public files directory in it, it will be stripped before
+ *    building the uri, since we're building a public:// uri
+ *
+ * @return
+ *    The path to the file as a uri.
  */
-function path_is_admin() {
-  if (arg(0) == 'admin') {
-    return TRUE;

Lastly, I don't think we need the function _bg_image_path_to_uri at all since D6 didn't use stream wrappers. We should just be able to use the $image_path directly in in the call to imagecache_create_url()

Does that all make sense?

Thanks!
drclaw

Cool, thanks for all your work on this so far!

However, I think we're not understanding each other on the AHAH/AJAX thing. Without your patch, the ajax works fine on the settings page. You select a node type, and the field list refreshes automatically. This uses AHAH which is built into core in D6. No dependencies.

It's only with your patch where the AHAH functionality is removed and D7 AJAX functionality is inserted that it breaks. I agree that we should remove the D7 AJAX stuff from the patch, but we should also retain the AHAH functionality that was there and working all along.

Additionally, I found a couple more D7-isms in your patch that won't work. They were both database query related. I've fixed those and updated the patch. It would be great if you could take a look when you get a chance.

Thanks!
drclaw

Hm, that's strange about the i18n stuff... I'm going to commit this patch for now, but hopefully I'll have a chance to explore it a bit further in the near future. If you figure anything out, maybe open a new issue?

Thanks again!
drclaw

Finally committed. Sorry for the wait!

https://drupal.org/commitlog/commit/23152/2884e2a080ec156647cc6aed75f7c6...