danf747 / geoxml3

Automatically exported from code.google.com/p/geoxml3
0 stars 0 forks source link

Missing markers when reloading #89

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Parse an url
2. Parse again to force reload

Version?
branches/kmz/r100

Description?
Two issues can face when reloading an url:
First, if two points have exactly the same coordinates, only first marker is 
activated. Second, activated markers are not added to the corresponding 
placemark (as is done when creating new markers).

The fix I have used for these issues are modifying lines 651-653 with these 
ones:
if (doc.markers[j].getPosition().equals(placemark.latlng) && 
!doc.markers[j].active) {
  found = doc.markers[j].active = true;
  placemark.marker = doc.markers[j];
  break;
}

Original issue reported on code.google.com by ava...@jsvrfid.es on 25 Jun 2013 at 10:37

GoogleCodeExporter commented 9 years ago
Do you have a test case for this functionality that you can attach or provide a 
link to  that shows the problem?

Original comment by geocodezip on 25 Jun 2013 at 2:18

GoogleCodeExporter commented 9 years ago
I'm afraid I can't, still developing in local and for a intraweb website with 
sensitive information. I can attach you some code to help you reproduce the 
problem... Basically, I've got a web page with a map and a side-list. When I 
click on a link, I load the kml file from a url:

 function loadMarkers(sender) {            //sender is a form used to filter
   var url_kml = $('#container).data('url-kml');
   var data = sender.data;
   myParser.parse(url_kml + '?' + data);
 }

When I click on elements on the side list, I open the infoWindow associated to 
the placemark with same 'id':

 function rowSelected(sender) {            
   var id = sender.data('id');
   var kml = sender.closest('li.active').data('url-kml');

   myParser.docsByUrl[kml].placemarks.forEach(function (e) {
     if (e.vars.val.id == id) {
       var m = e.marker;
       m.infoWindow.open(m.map, m);
     }
   });
 }

I noticed that first time works fine, but when clicking again, some markers in 
doc.markes where missing. also, started receiving errors after clicking on list 
elements.

Hope this code can help.

Original comment by ava...@jsvrfid.es on 25 Jun 2013 at 2:48

GoogleCodeExporter commented 9 years ago
Can you create a simple example with sanitized data and attach it to this 
issue? I will work on a test case, but don't have a lot of time right now.

Original comment by geocodezip on 25 Jun 2013 at 3:02

GoogleCodeExporter commented 9 years ago
Hi! I've created a simple test page. I attach you the html and the kml files.
To run the test, simply open the page and click the 'how many markers?' link. 
The alert will show that there are 6 markers, and checks if the first placemark 
has a marker object. Then click the 'reload' link and then, the 'kow many 
markers?' again. The alert now only shows 4 markers and the first placemark has 
no marker.

Hope this helps!

Original comment by ava...@jsvrfid.es on 26 Jun 2013 at 8:45

Attachments:

GoogleCodeExporter commented 9 years ago
Thank you for the test case.  Since you have multiple markers at the same 
location, seems like might need to look at the id of the placemark (if it has 
one, and these do), not the position so the correct one is activated.

Original comment by geocodezip on 26 Jun 2013 at 1:07

GoogleCodeExporter commented 9 years ago
That is what I thought in first place, but at that point, doc.placemarks only 
contains the new placemarks, due to initialiation starting in line 436.

Original comment by ava...@jsvrfid.es on 26 Jun 2013 at 2:13

GoogleCodeExporter commented 9 years ago
Online version of the testcase:
http://www.geocodezip.com/geoxml3_test/geoxml3-test.html

Original comment by geocodezip on 26 Jun 2013 at 5:14

GoogleCodeExporter commented 9 years ago
Temporary test version:
http://www.geocodezip.com/geoxml3_test/geoxml3-test_local.html

please test and provide feedback. added an id check (and added the id to the 
marker):

               doc.markers = doc.markers || [];
               if (doc.reload) {
                 for (var j = 0; j < doc.markers.length; j++) {
-                  if (doc.markers[j].getPosition().equals(placemark.latlng)) {
+                  if (doc.markers[j].getPosition().equals(placemark.latlng) &&
+                      (doc.markers[j]._placemarkID == placemark.id) && 
!doc.markers[j].active) {
                     found = doc.markers[j].active = true;
+                    placemark.marker = doc.markers[j];
                     break;

2 concerns:
1. what if the marker moves in the updated KML? (same id, different location; 
definitely not handled correctly in the current implementation)
2. what if the Placemark doesn't have an id?

What is your use case for reloading the KML (as opposed to 
hideDocument/showDocument)?

Original comment by geocodezip on 27 Jun 2013 at 6:07

GoogleCodeExporter commented 9 years ago
I though that the point of comparing the position of the placemark and marker 
was that it was the only way to perform the matching. If there is an id, such 
comparison is not needed in that case. So the modification may be something 
like (poor code, I know):

if (doc.reload) {
    for (var j = 0; j < doc.markers.length; j++) {
        if (doc.markers[j]._placemarkID != undefined) {
            if (doc.markers[j]._placemarkID == placemark.id) {
                found = doc.markers[j].active = true;
                placemark.marker = doc.markers[j];
                break;
            }
        }
        else {
            if (doc.markers[j].getPosition().equals(placemark.latlng) && !doc.markers[j].active) {
                found = doc.markers[j].active =? true;
                placemark.marker = doc.markers[j];
                break;
            }
       }
    }
}

For reloading KML, I simply call the same url again, but with a different 
string query. This results in the new KML file.

Original comment by ava...@jsvrfid.es on 28 Jun 2013 at 2:33

GoogleCodeExporter commented 9 years ago

Original comment by geocodezip on 30 Jun 2013 at 8:19

GoogleCodeExporter commented 9 years ago
Proposed change:
if (doc.reload) {
  for (var j = 0; j < doc.markers.length; j++) {
    if (((placemark.id && (doc.markers[j]._placemarkID == placemark.id)) ||
        (doc.markers[j].getPosition().equals(placemark.latlng)) && 
        !doc.markers[j].active) {
      found = doc.markers[j].active = true;
      placemark.marker = doc.markers[j];
      break;
    }
  }
}
Temporary test case:
http://www.geocodezip.com/geoxml3_test/geoxml3-test_local.html

Original comment by geocodezip on 15 Jul 2013 at 9:19

GoogleCodeExporter commented 9 years ago
fixed in revision 114 kmz branch.  Use id of marker if has one rather than 
position.  Issue was 2 markers had same position.

Original comment by geocodezip on 16 Aug 2014 at 5:33

GoogleCodeExporter commented 9 years ago
fixed in polys branch revision 115

Original comment by geocodezip on 16 Aug 2014 at 5:48