Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#11 | gmap-6.x-1.0.patch | 589 bytes | eliosh |
#9 | gmap-6.x-dev.module.patch | 587 bytes | jcmarco |
#5 | gmap_5.x-1.0.patch | 582 bytes | jcmarco |
#3 | gmap_5.x-1.0-rc1.patch | 877 bytes | jcmarco |
Comments
Comment #1
jcmarco CreditAttribution: jcmarco commentedThis 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 yourprint $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.
Comment #2
elioshPlease, 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.
Comment #3
jcmarco CreditAttribution: jcmarco commentedThe 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!
Comment #4
RobLoachIs there any reason why you're not using:
Comment #5
jcmarco CreditAttribution: jcmarco commentedHi 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.
Comment #6
RobLoachMuch better... This should possibly go into the DRUPAL-6--1 branch if it doesn't already exist as well.
Comment #7
RobLoachVery cool, thanks.
Comment #8
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedthis is great. thanks for this. it wasn't working for me until this patch. i hope it gets committed
Comment #9
jcmarco CreditAttribution: jcmarco commentedOnce 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
Comment #10
ron_s CreditAttribution: ron_s commentedI 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
inpage.tpl.php
, the GMap on the page never loads and I get a "Javascript is required to view this map" message.Comment #11
elioshConfirm Rob patch even form 6.x-1.0
Comment #12
jcmarco CreditAttribution: jcmarco commentedThe 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?
Comment #13
EvanDonovan CreditAttribution: EvanDonovan commentedI can confirm this works well for 6.x-1.1-rc1 also.
Comment #14
heyyo CreditAttribution: heyyo commentedWhy this patch is not in included in the module ?
Comment #15
RobLoachYup, still not committed.
Comment #16
mErilainen CreditAttribution: mErilainen commentedWorks 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.
Comment #17
RobLoachComment #18
ron_s CreditAttribution: ron_s commentedjcmarco, I have identified the issue. The patch is applied to the
theme_gmap
function. I changed this ingmap.module
, but I had forgotten I was already overriding this theme in mytemplate.php
file. As soon as I applied the patch intemplate.php
, everything works fine.Comment #19
ron_s CreditAttribution: ron_s commentedOne 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 incommon.inc
to ensurejquery.js
anddrupal.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?
Comment #20
seanrPatch works fine for me. Let's please get this committed.
Comment #21
seanrron_s, I think that should be a separate issue.
Comment #22
snorkers CreditAttribution: snorkers commentedTried 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.
Comment #23
becw CreditAttribution: becw commentedThis patch fixes this issue as well: http://drupal.org/node/266595
Comment #24
linuxuser CreditAttribution: linuxuser commentedI still have this problem.
There is a code in the middle, which is not mentioned above. When I removed this code, it didn't help.
Comment #25
linuxuser CreditAttribution: linuxuser commentedgmap-6.x-1.x-dev.tar.gz works for me!
Comment #26
liberatrThe patch from #11 worked for me as well.
Comment #27
gagarine CreditAttribution: gagarine commentedNot yet committed?
Comment #28
EvanDonovan CreditAttribution: EvanDonovan commented@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.
Comment #29
desecho CreditAttribution: desecho commentedI 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?
Comment #30
stfb CreditAttribution: stfb commentedSame problem as desecho ! Any guess ?
Comment #31
jcmarco CreditAttribution: jcmarco commentedDid you move the
print $scripts;
to just before
print $closure;
in your page.tpl.php theme template?
Comment #32
stfb CreditAttribution: stfb commentedYes I did. But still the same problem :-(
Comment #33
bdragon CreditAttribution: bdragon commentedhttp://drupal.org/cvs?commit=296318
http://drupal.org/cvs?commit=296316
Comment #34
desecho CreditAttribution: desecho commentedI 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.
Comment #35
nicholasThompsonI 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?
Comment #37
heyyo CreditAttribution: heyyo commentedEven after I applied the patch, I have always one gmap script in header:
And in footer I have
Is there any other patch to apply ?
My website:
http://www.rimonim-israel.com/node/132
Comment #38
heyyo CreditAttribution: heyyo commentedComment #39
bdragon CreditAttribution: bdragon commented@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.)
Comment #40
bowwowadmin CreditAttribution: bowwowadmin commentedI am getting the: Javascript is required to view this map. error
using 6.15 with the latest gmap and location modules.
Comment #41
mearns CreditAttribution: mearns commented@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.
Comment #42
focal55 CreditAttribution: focal55 commentedMake sure you have
print $head;
above your title tag if you are using a custom themeComment #43
EvanDonovan CreditAttribution: EvanDonovan commentedfocal55: even if you have applied this patch?
If it works with the patch, we should change back to "fixed".
Comment #44
sreynen CreditAttribution: sreynen commentedIt'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.
Comment #45
EvanDonovan CreditAttribution: EvanDonovan commentedSee bdragon's comment in #33. It has been committed to -dev.
Comment #46
Dublin Drupaller CreditAttribution: Dublin Drupaller commentedjust 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.
Comment #48
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedI 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
Comment #49
Jumoke CreditAttribution: Jumoke commentedJoining
Comment #50
calefilm CreditAttribution: calefilm commentedDublin 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.