WMEValidator / validator

WME Validator Source Files
GNU General Public License v3.0
11 stars 10 forks source link

Node attribute "partial" is buggy #64

Closed berestovskyy closed 5 years ago

berestovskyy commented 6 years ago

The "partial" attribute sometimes is not there.

Example 1: https://www.waze.com/fr/editor/?zoom=7&lat=51.73658&lon=3.82796&env=row&segments=343409108 Check: W.model.nodes.get(273196475).attributes

Example 2: https://www.waze.com/editor/?env=il&lon=34.80062&lat=31.24066&zoom=5&segments=791837,245357 Check: W.model.nodes.get(710253).attributes

Possible workaround: assign "partial" to all nodes outside the current view. Other ideas?

davidakachaos commented 6 years ago

But if the partial attribute isn't there, can we conclude from that that the node isn't a partial one? So shouldn't we see if the attribute is on the node, if so set the isPartial to it, else default to false?

berestovskyy commented 6 years ago

if the partial attribute isn't there, can we conclude from that that the node isn't a partial one?

That is exactly the bug. Sometimes there is no "partial" attribute, we conclude the node is complete, analyze it and show an error.

You can check it by running the listed command in the JS console. Open the permalink, run the command and see there is no "partial" attribute, but no enough connections. Move a map a bit, the "partial" attribute is still not there, but number of connections has increased silently...

davidakachaos commented 6 years ago

You can check it by running the listed command in the JS console. Open the permalink, run the command and see there is no "partial" attribute, but no enough connections. Move a map a bit, the "partial" attribute is still not there, but number of connections has increased silently...

I've tried that with the first example (second example is gone now, someone edited the map I think) and when I do, the segIDs of the node don't seem to change when I move the map around? I believe you when it happens on your machine, but it doesn't seem to happen here? Strange....

berestovskyy commented 6 years ago

For me both examples still work. The first one shows no "partial" attribute and 2 segIDs for node 273196475. This node is not selected in the permalink. How many segIDs do you see right after you open the permalink?

Once I move the view a bit up (northward), there are 3 segIDs, which is true.

davidakachaos commented 6 years ago

I see 3 segIDs. The PL does link to the French editor, which reloads the PL for me back to the Dutch editor. If I adjust the PL to use the NL editor (so no reload happens) it still shows me 3 segIDs... It's a strange thing, because I do suspect that this bug sometimes happens... The most frustrating bugs are the ones that depend on environment; maybe it's because I'm on a slow computer that takes a while to load things, and as such loads the node more completely? (Shot in the dark explanation right there...)

davidakachaos commented 6 years ago

Screen after load: image Viewing the node in the console: image

berestovskyy commented 6 years ago

Yeah, it's a bit tricky. There are few people already reported false positives for different checks, and I couldn't reproduce the issue as well...

I guess the node has to be further outside the screen. So first I open permalink in a new window, then open the JS console, which make the map view really small. Then I click permalink, which reloads the WME and I see the bug in 100% cases. I also checked different browsers -- the issue is there.

I can reproduce the bug, but it does not help much (or I can't spot the difference). The only idea I have at the moment is to check if a node is outside the screen and force mark it as partial...

davidakachaos commented 6 years ago

If it solves the false positives and doesn't mess with actual issues being reported, I think that would be a good enough option for now.

That is the difference in our testing; I open the JS console in a new window, not embedded in the browser window like you. That might be the difference why I don't see the bug (the node is on my screen) and you do (the node is off screen)

berestovskyy commented 6 years ago

I tried the following fix:

            this.$isPartial = n.attributes.partial;
+           if (!this.$isPartial) {
+               this.$isPartial = !WM.getExtent().
+                   containsBounds(n.geometry.bounds);
+           }

Unfortunately, it does not fix the issue for me. I move the map, the node becomes visible, but still has only 2 connected segments instead of 3 and no partial attribute...

berestovskyy commented 6 years ago

Now I have no highlights, but once I drag the map a bit northward, so the node is visible -- it is back 😞

screenshot 2018-10-01 at 08 20 09
berestovskyy commented 5 years ago

Fixed in WME v2.27-156