Problem with TileLayerOptions.

[evgenyt]

When you try to use TileLayerOptions as argument 'options' of L.tileLayer method (otherwise of null)

LeafletResources.whenReady( e -> {
    MapOptions options = new MapOptions.Builder(L.latLng(52.51, 13.40), 5.0, 5.0).dragging(false).build();
    final Map map = L.map("map", options);
    TileLayerOptions tloptions = new TileLayerOptions.Builder().attribution("test").build();
    L.tileLayer("http://{s}.tile.osm.org/{z}/{x}/{y}.png", tloptions).addTo(map);
    return null;
});

this provide next JavaScript error:

Uncaught TypeError: Cannot read property 'appendChild' of undefined
    at e._initContainer (leaflet.js:6)
    at e.onAdd (leaflet.js:6)
    at e._layerAdd (leaflet.js:6)
    at e.whenReady (leaflet.js:6)
    at e.addLayer (leaflet.js:6)
    at e.addTo (leaflet.js:6)
    at ENd_g$ (Main.java:31)
    at Function.LNd_g$ (Main.java:17)
    at HTMLScriptElement.lambda_0_g$ (Runtime.java:166)

The 'null' argument work correctly

L.tileLayer("http://{s}.tile.osm.org/{z}/{x}/{y}.png", null).addTo(map);

gwty-leaflet: 0.5 and 1.0-SNAPSHOT, GWT 2.8.0 and 2.8.1

[zak905]

Hi,

Thanks for reporting. I could reproduce the issue. I am trying to troubleshoot it

[zak905]

It turnouts the map pane should be created before using the TileLayer, so something like that works:

 final Map map = L.map("map", options);
    TileLayerOptions tloptions = new TileLayerOptions.Builder().pane("test").attribution("test").build();
    map.createPane("test");
    L.tileLayer("http://{s}.tile.osm.org/{z}/{x}/{y}.png", tloptions).addTo(map);

Sorry the method createPane was missing, I have just added it in the last commit. Maybe it is better to add it as an argument of the Builder to make it a required field.

You should be able to use it with 1.0-SNAPSHOT.

[ciaccia]

Hi all, I have experienced the same problem and I both figured out the reason and found a solution.

First: the solution is to patch the leaflet JS source:

1. In L.TileLayer initialize:

		if (typeof options.subdomains === 'string') {
			options.subdomains = options.subdomains.split('');
		} else if (Object.prototype.toString.call(options.subdomains) === '[object Array]' && options.subdomains.length == 1) {
			// BUG!!! JsInterOp
			options.subdomains = options.subdomains[0].split('');
		}

2. In L.Util setOptions:

	setOptions: function (obj, options) {
		if (!obj.hasOwnProperty('options')) {
			obj.options = obj.options ? L.Util.create(obj.options) : {};
		}
		for (var i in options) {
			// BUG!!! JsInterOp
			if (options[i]) {
				obj.options[i] = options[i];
			}
		}
		return obj.options;
	},

The reason for this is that the native JS object created by TileLayerOptions contains the keys for all the possible attributes with an undefined value. This is not what leaflet's API expect, therefore the default options get overwritten with undefined. The second patch above addresses this issue.

[zak905]

@ciaccia Thanks. It's an even better solution if it is handled by leaflet source Js. are you intending to submit the patch to leaflet Js project?

[ciaccia]

Hi Zak905,
Lots of APIs differentiate between an object that has a key with an undefined/null value and an object that has not key. We should not expect that a library changes its behaviour because of a local issue...

It is possible to fix the issue in GWT, using old "native" JavaScriptObject objects for the options instead of JsType. I have converted two lasses, everything works fine even without changing Leaflet's JavaScript.

https://www.dropbox.com/s/p7p89qitipe93t8/TileLayerOptions.java?dl=0
https://www.dropbox.com/s/e0z95djwpdodf24/MarkerOptions.java?dl=0

The trick is to generate JS objects without any "prototyped superclass", and to add the options to the object's internal map.

I forgot to mention that com.gwidgets.api.leaflet.events.Event getTarget() type is wrong. It should be JavaScriptObject instead of HtmlElement.

[zak905]

Hi @ciaccia,

Thanks for your remarks and your solution. I actually thought that this was a pure Leaflet Js bug, based on your previous answer. Off course, I would not expect a change to be made to Leaflet Js for fixing an issue here.

I actually was using JSNI before for all Options, and I decided to change to JsInterop to make everything 100% JsInterop. 898da51

Your solution seems reasonable, I will test it, and if it works out for everything, I will introduce the changes. I forgot that Js objects can be used as associative arrays.

and Yes com.gwidgets.api.leaflet.events.Event shoud be an object.

[zak905]

Hi @ciaccia,

I actually could not reproduce the bugs you pointed out.
I have tried putting breakpoints in setOptions method for TileLayer and a bunch of other objects, and the code executes properly without any bug. Can you please show some code snippet where this happens.
Concerning, the following:

	if (typeof options.subdomains === 'string') {
			options.subdomains = options.subdomains.split('');
		} 
// where did you get this piece of code because it does not exist in leaflet source code  
else if (Object.prototype.toString.call(options.subdomains) === '[object Array]' && options.subdomains.length == 1) {
			// BUG!!! JsInterOp
			options.subdomains = options.subdomains[0].split('');
		}

I am sure that the else part does not exist in leaflet source code version 1.0 and 1.0.1. where did you find it?

Unless we can reproduce the bugs clearly, I will not be able to integrate your solution, sorry.

The solution above still holds for the bug reported by @evgenyt

[ciaccia]

Hi @zak905,
If you try the following code in GWT, you will see that setOptions is called with a TileLayerOptions object having (for example) pane set to undefined

MapOptions options = new MapOptions.Builder(L.latLng(47.0, 7.0), 17.0, 7.0).maxZoom(17.0).build();
Map map = L.map("myMapId", options);
TileLayer l = L.tileLayer("https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png",
    new TileLayerOptions.Builder().minZoom(1).maxZoom(17).build());
l.addTo(map);

This will then overwrite the default value of tilePane.

The second point if not relevant anymore. The else if (Object.prototype.toString.call(options.subdomains)... block was added by me to overcome the problem with the default values overwritten (see first point). More specifically, leaflet has a tricky behaviour for the subdomains property. The default value is the string "abc", then it is converted into an array ["a", "b", "c"]. Your JsInterop wrapper passed a subdomains with a value of ["abc"] which broke the functionality.

[zak905]

Hi @ciaccia,
the code you provided does not lead to any error or exception. The goal of the setOptions method is to build an options literal object with the provided values by the user, merged with the default values, as described in the documentation:

Merges the given properties to the options of the obj object, returning the resulting options. See Class options. Has an L.setOptions shortcut.

I used a break point in setOptions in Chrome and it produces the expected output. Unless you can give a use case with an error stack trace, I will not be able to qualify this as a bug.

[ciaccia]

Hi zak905,

Let's say you use your "normal" code (not the one with the JNSI trick I provided) and you execute the following snippet:

MapOptions options = new MapOptions.Builder(L.latLng(47.0, 7.0), 17.0, 7.0).maxZoom(17.0).build();
Map map = L.map("myMapId", options);
TileLayer l = L.tileLayer("https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png",
    new TileLayerOptions.Builder().minZoom(1).maxZoom(17).build());
l.addTo(map);

When the JsInteroped TileLayerOptions is passed to native leaflet's setOptions, how many properties do you see? More specifically, the for (var i in options) { loop how many times gets executed? On my PC with my GWT, it was executed for every possible property defined in TileLayerOptions, even if not set. What happens with your setup?

Edit: just to be on the safe side, I never mentioned an exception or a stacktrace happening in setOptions. I have just said that default values were overwritten with undefined.

[zak905]

@ciaccia,

Would you like to submit a PR ?

[zak905]

@ciaccia nevermind.

I found a better solution to the problem, without polluting the code base with JSNI. All I needed to do is to remove all default values ( they are provided by .js) and check if the value is null before making an assignement in the builder e.g: https://github.com/gwidgets/gwty-leaflet/blob/master/src/main/java/com/gwidgets/api/leaflet/options/TileLayerOptions.java

@evgenyt You can use the title options objects now without having to use map.createPane("test");
Please let me know if this works out, so that we close this issue.

[ciaccia]

Hi @zak905 ,
When trying to understand the underlying issue for this bug I played a bit with JsInterop and I thought a default undefined value was always assigned to a field annotated with @JsProperty, there fore I suggested the old JSNI solution.

I still have to test your commit, but if what you say is correct you found the right fix for the issue! In case I spot something else I will get back to you.

Cheers and thanks