Related
#1167594: Saving gmap_markers.js needs catch errors
#1620060: git reports gmap_markers.js has been modified
#923630: Markers not showing up on Gmaps no matter what: How to debug
#1153032: Markers doesn't work in a view with the configuration: choose latitude and longitude fields
#1924924: Javascript files needed for a GMap are being added twice
#1218870: Marker managers give wrong path to marker icon when Drupal is not installed at root
#315236: GMap View, Exposed Filters and the error "Javascript is required to view this map" upon AJAX update
#1884604: Gmaps view not displaying related nodes
#1580330: Using custom field for Marker Field results in markername getting set to the node id
#1941530: gmap_markers.js not found
ToDo
Code coverage with tests
the test should include
- 2 testing views with gmap style plugins
- testing all js files for access (http status 200) on view page
- testing views plugins forms via nojs get requests
Follow-Up
Comment | File | Size | Author |
---|---|---|---|
#61 | gmap-markers-1931138.patch | 792 bytes | kenorb |
#13 | gmap2_1931138_13_fix_all_markers.patch | 10.75 KB | johnv |
#10 | gmap2_1931138_10_fix_all_markers.patch | 10.75 KB | johnv |
#7 | gmap2_1931138_7_fix_all_markers.patch | 10.88 KB | johnv |
#2 | markers7.x-1.x.png | 571.02 KB | podarok |
Comments
Comment #1
podarokthe same for 7.x-1.x
http://drupalcode.org/project/gmap.git/commitdiff/314fa02
Comment #2
podarokhere is a screenshot for 7.x-1.x release
Comment #3
podaroktag
@todo - fix relative issues
Comment #4
johnvSome test-remarks:
It does this also for https://ditu.google.cn/maps/api/js?v=3&language=en&sensor=false
As it is a Chinese version, no https is supported. Only thát link uses hardcoded http://
A bunch of .js files is removed from gmap_gmap, since they are already always loaded in _gmap_base_js()/hook_element_info().
This makes the function _gmap_doheader() superfluous, too.
Also, all javascript is now added via Form API, not drupal_add_js.
$GLOBALS['base_url'] . '/' . variable_get('file_public_path', conf_path() . '/files') . '/js/gmap_markers.js'
breaks my site: the result of this call is http:// , but my browser fetches https://, giving an red error:GET https://example.com:8082/sites/all/files/js/gmap_markers.js
So, I set it back to
'/' . variable_get('file_public_path', conf_path() . '/files') . '/js/gmap_markers.js'
, which makes me happy :-)@podarok, this may upset someone, or you. But let's only change this for documented cases (http vs https, subdomains...
gmap_gmap($op, &$map) , case '$op == pre_theme_map'
contains a sections that adds js.This is removed/moved, since this functionality is already in the 'official' hook _gmap_pre_render_map() and callback _gmap_base_js .
I removed it and added 'gmap_views_ajax.js' via _gmap_base_js().
Some things must still be tested:
- Two displays on the same page with different markertype (e.g, cluster vs. non-cluster)
Comment #5
podarok#4 please, provide a patch for that ))))
Comment #6
johnvYes, I'll create a patch.
First, I will log my errors on testing, editing #4 to not create a long list of comments.
Comment #7
johnvPlease find attached patch.
IMO only #1167594: Saving gmap_markers.js needs catch errors remains.
I concentrated on two things:
- centralize addition of javascript, in the 2 'official' hooks/callbacks: _gmap_base_js() and _gmap_pre_render_map().
- remove unused code. I merely uncommented this, so other people can easily see what is removed and why. We should get rid of these parts as soon as the version stabilizes.
- the
variable_get('file_public_path' ...
issue. It works on my site. I invite others to test on theirs and report the findings, together with their constellation.Comment #8
podaroktrailing whitespace http://drupal.org/coding-standards#indenting
Another issue for creating such patches - http://drupal.org/node/1354#code if You commenting a code - You should use or code samples or @todo with rer line comments for easy patch reviewing
Bad example
Preffered style
It is still needs work
We shouldn`t commit any part of code with comments and debugs instead of the @todo http://drupal.org/node/1354#todo with links to real issues or @deprecated with links to real change-records.
But this patch is good enouph for a Needs manual testing, so tagging
Comment #9
johnvSo, what do you prefer? Shall I remove all obsolete code?
Comment #10
johnvA clean patch.
Comment #11
johnvtestbot.
Comment #13
johnvnew version.
Comment #14
drtrueblue CreditAttribution: drtrueblue commentedHere is another marker issue that was closed, won't fix. It was requested that I retest the issue with the new version. Unfortunately, it is still not resolved. My apologies if this should be a separate issue from this current thread, I was trying to help concentrate the same type of issues. Thanks for your review and support.
https://drupal.org/node/1884604
Comment #14.0
johnvUpdated issue summary. Added Related
Comment #15
johnv@drtrueblue, no, it is not resolved yet in the latest -dev version (1.x nor 2.x). But it is after applying this patch.
I did a test for a view with locations on a related node/entity, and the view was OK.
So can you check your case, and report back? :
- install 7.x-2.x, apply this patch #1931138-13, flush caches.
Comment #15.0
podarokUpdated issue summary.
Comment #15.1
podarokUpdated issue summary.
Comment #16
podarokwe shouldn`t leave such comments with code
If You want to be descriptive, just type a small description why this changed in comment with patch or(and) in function phpdoc if this is usefull for DX
#13 looks good
but we still needs tests for that and #1931138-20: Fixing all 'single marker' issues
the test should include (updating summary)
Comment #17
podarokYes - preffered style - removing obsolete code
After pushing such patch to repo we should create change-record with
before
old code example
after
new code example
examples
PS.tag API change
Comment #18
johnvHi podarok, unfortunately, I cannot help you with automated tests.
Comment #19
podarok#18
I`d planned this for myself with examples for You or someone else
With tests we can much easier implement new features and fix bugs without generating new ones
It`s easy...
Comment #20
podarokBTW important addition for #13 review
if we removing the function we should leave wrapper for minor release update dependency stability
needs work
function
_gmap_doheader()
should be leaved with wrapper code and @deprecated in commentsExample
This is easy and useful for those modules and especially tests!!! that possibly used this function and do not brakes their functionality in minor gmap release update
Comment #21
podaroktag
Comment #22
johnvRegarding the deletion of _gmap_doheader():
IMO functions with a preceding '_' are supposed to be internal functions, and not to be used by other modules.
But indeed, better be cautious, to not break other modules.
We might add a watchdog message, to give some warning.
Comment #23
podarokYes, this is true
But, due to bad code coverage with tests and partially abandoned(400+ bug issues)/bad-api-descripted gmap module before @podarok->maintainer we should be painless for those developers that do reverse engineering and integrated their work with current implementation
Comment #24
podaroknope, @deprecated + change-record is a good workflow
these functions will be
highlighted as strikein most IDEComment #25
podaroktag
Comment #26
podarok#13: gmap2_1931138_13_fix_all_markers.patch queued for re-testing.
Comment #27
podarokpatch pass tests, but needs reroll due to #16 and #20
Comment #27.0
podarokUpdated issue summary.
Comment #28
johnvAttached patch is a reroll of #13, including #16.
You might want to add the documentation in #20 yourself.
Comment #29
johnvnew try.
Comment #30
podarok#29 commited pushed to 7.x-2.x-dev
tagged
Comment #31
podaroksome docs and DX info
Comment #32
podaroktrailing whitespace will be removed before commit))
Comment #33
podarok#31 commited pushed to 7.x-2.x
Comment #34
drtrueblue CreditAttribution: drtrueblue commentedThank you for the time and effort. I apologize for my somewhat novice experience level, but I'm not skilled at applying patches. It's reassuring to know you've put forth a good fix that you've tested and I'll patiently await the next formal update. Again, thank you for the consideration.
Comment #35
podarok#34 this patch already commited
You can try 7.x-2.x-dev release
Comment #35.0
podarokUpdated issue summary.
Comment #36
johnvThis patch needs a follow-up to use the correct URL's:
https://developers.google.com/maps/documentation/geocoding/#GeocodingReq...
Comment #37
mapee CreditAttribution: mapee commentedhello,
i updated gmap from v. 7. 2.4 to v. 7. 2.5 beta1, but all markers not showing up ( the map become empty) i uploaded markermanager.js and markermanager_packed.js to thirdparty folder
any help?? iam using GMaps Utility Library MarkerManager
Comment #38
natureofdorset CreditAttribution: natureofdorset commentedI have just upgraded to 7.2.4 and my markers no longer display. I went further and uploaded 7.2.5-beta1 but no change. The log file shows 'page not found' and default/files/js/gmap_markers.js
Help, please, asap!
Comment #39
edvanleeuwenIt might be that the wrong path is used. See http://drupal.org/node/1937990. To test: look into your log report and verify the path with the setting. If those differ, you could revert to a previous version or wait until this issue is solved.
Comment #40
podaroknow this is a latest pre-stable version
Comment #41
mapee CreditAttribution: mapee commentedsame problem on gmap 2.6 - drupal 7 , no markers dispaly, please any help,
but on the wrong version (2.5) it works
Comment #42
johnv@mapee, please check your Browser developer tools, like Chrome | Extra | Developer tools, and check for (javascript) errors.
Comment #43
mapee CreditAttribution: mapee commentedthanks johnv
i tried for many browsers like chrome , firefox , safari , internet explorer, in the end the same problem,
you can see this link for multiple markers
http://www.mapee.net/new/
for single marker:
http://www.mapee.net/new/node/221
i am using gmap extended
Data Source: choose Latitude and Longitude fields
Comment #44
johnv@mapee, I mean this messag in the Developer tools:
GET http://www.mapee.net/sites/default/files/js/gmap_markers.js 404 (Not Found)
Does the fiel exist on the server? Did you generate the markers, using admin/config/services/gmap?
Comment #45
mapee CreditAttribution: mapee commentedthank you very much johnv,
as you said, the problem was gmap_markers.js (Not Found)
i correct it.
thanks again
Comment #46
podaroklooks like all are fixed
feel free to reopen if not
Comment #47
johnvChanging title, since the following set of issues will be "Fixing all Clustering issues" (due to APIv2 -> APIv3 conversion), like:
#1954304: MarkerClusterer not work, give JS error.
#1968416: Markers disappear when using Jef Poskanzer's Clusterer (due to Google API v3)
Comment #49
Fly85 CreditAttribution: Fly85 commentedHi, I have the same problem with gmap 7.x-2.6, I changed to 7.x-2.7-rc1+3-dev and still have no markers displayed.
I have one javascript error :
I used the code from the help folder to display a map in a node. From my template.php :
The map is displayed but there are no markers.
-- edit --
By adding
in the marker array fixed it. Sorry for reopening the ticket.
Comment #50
johnv@Fly85, so you are programmatically adding a map? If so, please open a new issue. Perhaps the 'code from the help folder' is outdated.
Comment #51
Garra CreditAttribution: Garra commentedHello
i have problem to display marker
The marker is displaying when the user select the addresse directly in the map
but it does not display when VIEW node.
I have 7.x-3.0-rc3 version of location and 7.x-2.7 of gmap (7.23 Drupal version)
i'd tested with location as field and with location in node locations.
both the same problem
could anybody help me???
thanks a lot
Comment #52
drupalina CreditAttribution: drupalina commentedGarra, I have been struggling with this issue in D7 for many months (probably more than a year). see this for example: https://drupal.org/node/1552238
Have you found any solution to get this to work?
Does anybody know how to make GMap display the pinpoint from the address provided in Location field in D7?
Comment #53
kenorb CreditAttribution: kenorb commentedThe same problem with Dev version.
This file exists in: ./sites/default/files/gmap_markers.js
But gmap is looking in sites/default/files/js/gmap_markers.js
Comment #54
kenorb CreditAttribution: kenorb commentedIt happens especially during the upgrade of gmap module, so the previous file probably was in different place and the new file is suppose to be in public://js/gmap_markers.js
But
file_save_data($contents, 'public://js/gmap_markers.js', FILE_EXISTS_REPLACE);
works like this, if the file exists, then it points to the existing path which was generated before.And checking this line:
it seems the path is hardcoded despite of saved gmap_marker_file fid.
I think we should check and load the existing filepath like:
WORKAROUND
For workaround, I'm suggesting:
And regenerate gmap markers at /admin/config/services/gmap or by clearing the cache from UI (as in CLI it doesn't work).
If you'll have still some problems, probably you have more than one file for gmap markers. You can see them all by the following command:
You can remove these file by (ignore the first errors):
Comment #54.0
kenorb CreditAttribution: kenorb commentedUpdated issue summary.
Comment #55
kenorb CreditAttribution: kenorb commentedFor people who're using Media Browser Plus, there is another issue:
#2085857: media_browser_plus_move_file() breaks the JS paths in GMap
Comment #56
drupalina CreditAttribution: drupalina commentedFor whatever it's worth, I got the markers to show by replacing the entire gmap-2.7 with the latest .dev version.
Comment #57
kenorb CreditAttribution: kenorb commentedComment #58
podarokGood to see here simpletest test
Comment #59
kenorb CreditAttribution: kenorb commentedFew issues I've found with the #57 (like all to a member function getExternalUrl()), so my better proposal is:
based on the patch code.
I'll provide updated patch soon.
Comment #60
hutch CreditAttribution: hutch commentedThe directory "files/js" is used by Drupal's javascript aggregation which has in my experience deleted everything in there when being reset or unset, although gmap_markers.js will be rebuilt at the next cron job it is possible for gmap to find itself without a markers file for a while. The solution in my opinion is to store gmap_markers.js in its own directory under files, eg "files/gmap/gmap_markers.js"
Comment #61
kenorb CreditAttribution: kenorb commentedComment #62
podarokI can`t push it without testing coverage
Comment #63
kenorb CreditAttribution: kenorb commentedMarked as duplicated of this: #2086529: Location marker is not shown
Comment #64
jami3z CreditAttribution: jami3z commentedWhen applying #61, I get the following error when trying to regenerate the marker cache
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'public://js/gmap_markers.js' for key 'uri': INSERT INTO {file_managed} (uid, filename, uri, filemime, filesize, status, timestamp, type) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7); Array ( [:db_insert_placeholder_0] => 2 [:db_insert_placeholder_1] => gmap_markers.js [:db_insert_placeholder_2] => public://js/gmap_markers.js [:db_insert_placeholder_3] => application/x-javascript [:db_insert_placeholder_4] => 4340 [:db_insert_placeholder_5] => 1 [:db_insert_placeholder_6] => 1385349684 [:db_insert_placeholder_7] => undefined ) in drupal_write_record() (line 7154 of /xxxxxxxx/docroot/includes/common.inc).
Comment #65
podarokdue to #64 the patch #61 needs work
Comment #66
kenorb CreditAttribution: kenorb commentedComment #67
spidersilk CreditAttribution: spidersilk commentedI applied the patch from #61 a few months ago, along with the one from #2185395: Use managed file table to retrieve gmap_markers.js, because it took both to get the markers showing. And everything seemed fine... Until today, when I got the error in #64.
But I didn't get it while trying to specifically do anything to the marker cache, or even while doing anything involving Gmap at all - it happened when I was trying to save changes from the Modules page (installing a new module, unrelated to Gmap). And there were a few other oddities - the duplicate URI referred to was 'public://gmap_markers_942.js', and when I checked the files directory, it looked like the markers file had been replicating itself over and over, so that there were now 1003 copies of it! With corresponding entries in the files_managed table. I did file comparisons between a few of them, and they were all identical. 1003 copies of the same file!
I have no idea why it was trying to insert version 942 a second time, or why it was trying to do anything with the map markers on an admin page that had nothing to do with the maps, but it seems like it might be related to this patch... Though the site is also using Media Browser Plus, in case that's relevant.
Another oddity: while there is a folder called gmap inside the files folder, these files aren't being generated there - they're just in the files folder. The gmap folder is empty.
Any ideas?
Comment #68
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedI would like to add private file system markers to the Meta list:
https://www.drupal.org/node/1940996
Comment #69
jomarocas CreditAttribution: jomarocas as a volunteer commentedhi i update and issue that the status is active
Comment #70
jomarocas CreditAttribution: jomarocas as a volunteer commentedSorrty, work at expect if you choose lat and lon fields
Comment #71
kenorb CreditAttribution: kenorb commented