Based on this article :
http://wimleers.com/article/improving-drupals-page-loading-performance
I put all JSes on bottom of page.

I can see that gmap does not work, because module write script tag directly in body.

So i copy and paste theme_gmap function :


// ...

  // $map can be manipulated by reference.
  foreach (module_implements('gmap') as $module) {
    call_user_func_array($module .'_gmap', array('pre_theme_map', &$map));
  }

  // Inline settings extend.
  $o .= '<script type="text/javascript">'."\n";
  $o .= "/* <![CDATA[ */\n";
  $o .= 'Drupal.extend({ settings: '. drupal_to_js(array('gmap' => array($element['#map'] => $map))) ." });\n";
  $o .= "/* ]]> */\n";
  $o .= "</script>\n";
  
  return $o;

making this substitution :


  // $map can be manipulated by reference.
  foreach (module_implements('gmap') as $module) {
    call_user_func_array($module .'_gmap', array('pre_theme_map', &$map));
  }

  // Inline settings extend.
  $o_script .= 'Drupal.extend({ settings: '. drupal_to_js(array('gmap' => array($element['#map'] => $map))) ." });\n";
  
  drupal_add_js($o_script, 'inline');
  
  return $o;

I think it'll be great to put this modification in module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jcmarco’s picture

This patch works really well!

As Eliosh says, it is better for performance to load all javascript at the end of the html, so if you want to increase the load performance, just change the page.tpl.php template of your theme in order to load the scripts at the end (moving print $scripts; just before your print $closure; )

Also IMHO, it is more of a Drupal standard to load the JS with drupal_add_js function.

Actually that is mandatory if you are working with javascript_aggregator or sf_cache modules.

eliosh’s picture

Version: 5.x-1.0-beta3 » 5.x-1.0-beta5

Please, can someone confirm me that this is a possible patch for future versions ?
I made this patch on a production site and, after upgrading from beta3 to beta5, i spent hours to understand why it doesn't work (without the patch).

Thanks a lot for all your work.

jcmarco’s picture

Version: 5.x-1.0-beta5 » 5.x-1.0-rc1
Status: Needs review » Reviewed & tested by the community
FileSize
877 bytes

The patch still works great also with 5.x-1.0-rc1!
Works fine with or without JS aggregation module or changing the JS load order in the page.tpl.php
More performance!

RobLoach’s picture

Is there any reason why you're not using:

drupal_add_js(array('gmap' => array($element['#map'] => $map)), 'setting');
jcmarco’s picture

Version: 5.x-1.0-rc1 » 5.x-1.0
Status: Reviewed & tested by the community » Needs review
FileSize
582 bytes

Hi Rob,
I was applying the eliosh recommendation and it helped to understand the change. I agree with you that your code is cleaner.

I have re-rolled the patch removing old lines and leaving only the drupal_add_js. I have tested it and works fine.

Just as a reminder, this change allows that Drupal manage the javascript, allowing that if you are using JS Aggregator to optimize the html and js loading times GMap will work fine.

RobLoach’s picture

Version: 5.x-1.0 » 5.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

Much better... This should possibly go into the DRUPAL-6--1 branch if it doesn't already exist as well.

RobLoach’s picture

Very cool, thanks.

SocialNicheGuru’s picture

this is great. thanks for this. it wasn't working for me until this patch. i hope it gets committed

jcmarco’s picture

Title: Put javascript on bottom of the page (as suggested by YSLOW) » Patch to use JS Aggregation or JS at the bottom of the page
Version: 5.x-1.x-dev » 6.x-1.x-dev
Status: Reviewed & tested by the community » Needs review
FileSize
587 bytes

Once again for GMap for Drupal 6.x.

If your theme has the js at the bottom of the page, or you are aggregating JS, you need this to work

ron_s’s picture

I applied the patch, and it worked fine (in 5.x-1.1-rc1). However if I move $scripts to the bottom of the page right before $closure in page.tpl.php, the GMap on the page never loads and I get a "Javascript is required to view this map" message.

eliosh’s picture

Version: 6.x-1.x-dev » 6.x-1.0
Status: Needs review » Reviewed & tested by the community
FileSize
589 bytes

Confirm Rob patch even form 6.x-1.0

jcmarco’s picture

The patch works fine. And it adds the feature to be aggregated to the bottom of a page, increasing load performance. It is also more drupalish than original code.

Is there some issue with that patch that we could help to solve and avoid to being patching every time that GMap module is updated?

EvanDonovan’s picture

Version: 6.x-1.0 » 6.x-1.1-rc1

I can confirm this works well for 6.x-1.1-rc1 also.

heyyo’s picture

Why this patch is not in included in the module ?

RobLoach’s picture

Version: 6.x-1.1-rc1 » 6.x-1.x-dev

Yup, still not committed.

mErilainen’s picture

Works well for me also. I had problems when using with Panels, that randomly the maps were just missing, without any message. Now it works fine.

RobLoach’s picture

Priority: Normal » Critical
ron_s’s picture

The patch works fine. And it adds the feature to be aggregated to the bottom of a page, increasing load performance. It is also more drupalish than original code.

jcmarco, I have identified the issue. The patch is applied to the theme_gmap function. I changed this in gmap.module, but I had forgotten I was already overriding this theme in my template.php file. As soon as I applied the patch in template.php, everything works fine.

ron_s’s picture

One additional comment: I had attempted to apply this patch suggested by Wim Leers to improve Drupal performance. The patch modifies the drupal_add_js function in common.inc to ensure jquery.js and drupal.js are loaded at the bottom of the page.

When using this patch with GMap, the map is loaded, but no markers are shown. I have looked at the code extensively, and all markers are being generated as expected. However when the map is built on the page, no markers exist (not even a marker "shell" in the HTML).

Any thoughts?

seanr’s picture

Patch works fine for me. Let's please get this committed.

seanr’s picture

ron_s, I think that should be a separate issue.

snorkers’s picture

Tried the patch and moved $scripts to the foot of page.tpl.php. Works fine in a Blueprint based theme; although in Admin/Slate part of the map is missing - looks like it is rendering only upper left tile of the map. Tried in FF3 and Saf4 on a Mac.

becw’s picture

Assigned: Unassigned » bdragon

This patch fixes this issue as well: http://drupal.org/node/266595

linuxuser’s picture

I still have this problem.

sites/all/modules/gmap/gmap.module:

  // $map can be manipulated by reference.
  foreach (module_implements('gmap') as $module) {
    call_user_func_array($module .'_gmap', array('pre_theme_map', &$map));
  }

  if (isset($mapids[$element['#map']])) {
    drupal_set_message(t('Duplicate map detected! GMap does not support multiplexing maps onto one MapID! GMap MapID: %mapid', array('%mapid' => $element['#map'])), 'error');
    // Return the div anyway. All but one map for a given id will be a graymap,
    // because obj.map gets stomped when trying to multiplex maps!
    return $o;
  }
  $mapids[$element['#map']] = TRUE;

  // Inline settings extend.
   $o_script .= 'Drupal.extend({ settings: '. drupal_to_js(array('gmap' => array($element['#map'] => $map))) ." });\n";
 
  drupal_add_js($o_script, 'inline');
 
  return $o;
}

There is a code in the middle, which is not mentioned above. When I removed this code, it didn't help.

linuxuser’s picture

gmap-6.x-1.x-dev.tar.gz works for me!

liberatr’s picture

The patch from #11 worked for me as well.

gagarine’s picture

Not yet committed?

EvanDonovan’s picture

@gagarine: No - we need to find a way to get in touch with the maintainer (bdragon). He hasn't been replying in the issue queue for the past few months.

desecho’s picture

I have applied the patch #11, but the javascript error is still there. I can see the script tag and "CDATA" text in the source. It looks like it has not been updated, but the module is definitely patched and I cleared the cache using in Admin->Performance menu. Does anyone have a guess what might be a problem?

stfb’s picture

Same problem as desecho ! Any guess ?

jcmarco’s picture

Did you move the
print $scripts;
to just before
print $closure;
in your page.tpl.php theme template?

stfb’s picture

Yes I did. But still the same problem :-(

bdragon’s picture

Status: Reviewed & tested by the community » Fixed

http://drupal.org/cvs?commit=296318
http://drupal.org/cvs?commit=296316

  #286653: Patch to use JS Aggregation or JS at the bottom of the page by jcmarco et al: Use drupal_add_js() like everyone else.
  
  HOPEFULLY it will work okay.
  The reason I was reluctant to change this is the interactions between drupal_add_js()
  and hook_filter(), described at
  http://mostrey.be/hookfilter-versus-drupaladdjs
  
  If anyone has troubles with macros after this change, I will have to revisit gmap's filter
  stuff.
desecho’s picture

Status: Fixed » Active

I have installed the latest dev version but nothing's changed. It still displays the javascript error instead of the map after the search is submitted.

nicholasThompson’s picture

Status: Active » Fixed

I have $scripts being printed at the end (before $closure) with JS aggregation off.

The latest dev works perfectly. Initially (with 1.1-RC) I was having issues with the macro builder.

Could I please request a tagged release of GMaps?

Status: Fixed » Closed (fixed)

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

heyyo’s picture

Even after I applied the patch, I have always one gmap script in header:

<script src="http://maps.google.com/maps?file=api&amp;v=2.115&amp;key=ABQIAAAATYf5ID3lwSK4Prt2s9tFsRS0-gt7Uwa6UzpvTX7SuJvFDkwPcBTmQl4ISiiorwxKn9zagcXaxXjzSA&amp;hl=en" type="text/javascript"></script>

And in footer I have

<script type="text/javascript" src="/sites/default/files/js/gmap_markers.js?F"></script>

Is there any other patch to apply ?

My website:
http://www.rimonim-israel.com/node/132

heyyo’s picture

Status: Closed (fixed) » Active
bdragon’s picture

@heyyo:

The gmap_markers.js in the footer is not aggregatable because it is something that is computed and stored in the files folder. At least it's in the footer now ;)

The maps.google.com one is stuck in the header at the moment because I'm using drupal_set_html_head() to add it. This is subject to change, especially for Drupal 7 where including external javascript is easier. (Of course, as it is an external file it is DEFINATELY not aggregatable.)

bowwowadmin’s picture

I am getting the: Javascript is required to view this map. error
using 6.15 with the latest gmap and location modules.

mearns’s picture

@bowwowadmin make sure you're using the (latest) dev release of the gmap module (http://ftp.drupal.org/files/projects/gmap-6.x-1.x-dev.tar.gz) ... I was getting this error using the release candidate/stable version and newest installs of all related mods, but now all is good.

focal55’s picture

Make sure you have print $head; above your title tag if you are using a custom theme

EvanDonovan’s picture

focal55: even if you have applied this patch?

If it works with the patch, we should change back to "fixed".

sreynen’s picture

Status: Active » Reviewed & tested by the community

It's only fixed when the patch is applied. People have said the patch fixes it in both this issue and duplicates, so I'm marking this as reviewed & tested by the community.

EvanDonovan’s picture

Status: Reviewed & tested by the community » Fixed

See bdragon's comment in #33. It has been committed to -dev.

Dublin Drupaller’s picture

just to confirm..I was having the same problem but the gmap-6.x-1.x-dev version is working and seems to solve the issue that throws up the "Javascript is required to view this map" message.

Status: Fixed » Closed (fixed)

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

SocialNicheGuru’s picture

I am cross posting because I had a similar issue and took me a few hours to find

I had a specific issue of javascript error when I had a gmap in a panel page. It ended up being a javascript error caused by ctools.

the solution for me is here: http://drupal.org/node/888254

Jumoke’s picture

Joining

calefilm’s picture

Dublin in #46, I just had the dev version enabled and was getting the message: "Javascript is required to view this map"

SocialNicheGuru in #48, I have the current ctools enabled so I know it can't be that module conflicting.

I'm going to re-enable the dev version.. as I just disabled Gmaps tools . .maybe there was some conflict there.

Update: I updated to dev version again and created a new Google Map API key (don't think I needed to do this) and got it working.. I went back to 6.1 just to confirm the issue again and it re-occurred so i guess i have to stick with dev. I still get the Javascript error message with 6x.1 when the AJAX is turned on and i filter it to another day. In 6.x-dev I can turn AJAX on and so long as nodes exist under the initial page load I don't see the javascript error message.

my view w/ AJAX explanation:

I am attaching a view onto a panel. In order to achieve getting at fields that i couldn't otherwise get to, I created a display: Attachment that I attached to another display. Regardless, on my view, I added a filter so that I could sort through nodes based on the date the user inputted.

In order to sort by date I turned on AJAX in the page display. So the problem was, the user would go to this panel page.. and if no view displayed for that particular date, the view showed the user the initial page-load using the AJAX feature, which makes that one URL the one and only.. and when the user selects another date, nothing happens except I get the Javascript error message)... thus, if I want to use the AJAX feature I have to make sure a node has been created for that date already.. otherwise, I have to turn AJAX off.