Id: Redoing a deletion does not refresh rendering

Created on 3 Jun 2019  路  7Comments  路  Source: openstreetmap/iD

Reproduction:

  1. Draw an area or select an existing area
  2. Delete the area
  3. Undo the deletion
  4. Redo the deletion (notice the area is still drawn)
  5. Select the area and it will disappear
bug map-renderer

Most helpful comment

Ok. Perhaps I got ahead of myself and did not actually find the bug above.
After more debugging, I think this is the actual issue.

You do an undo, then a code path leads you to the drawEditable function in the map renderer.
There was a change made here
when lodash was removed.

In a scenario where there is a (difference)[https://github.com/openstreetmap/iD/commit/d5abe468b97afd2a4186ef43d488891c5cca2c63#diff-941aa7b2c44d2d015cb63f82424da372R276-R283], we now do set.has(d.id) to filter features, where we used to do, d.id in completed.

The old way returns true for recently deleted features. But, the former will return false because this set has already filtered out the undefined entities.

What if we try the old filter?

All 7 comments

The behavior seems not only exist for areas. I noticed similar behavior for lines and nodes.

I get this to happen when I add 2 features and undo until I get to the base of the undo/redo stack (like undo everything I did).

I put a logpoint in the undo.map event handler here.

Here's my browser console.

image

The conditional doesn't redraw when we are in browse mode. Indeed, when I reproduce, I am in browse mode.

Also for a sanity check, I logged out the graph and the only feature in the graph is null island!
I guess I should see why we are in browse mode?

Ok. Perhaps I got ahead of myself and did not actually find the bug above.
After more debugging, I think this is the actual issue.

You do an undo, then a code path leads you to the drawEditable function in the map renderer.
There was a change made here
when lodash was removed.

In a scenario where there is a (difference)[https://github.com/openstreetmap/iD/commit/d5abe468b97afd2a4186ef43d488891c5cca2c63#diff-941aa7b2c44d2d015cb63f82424da372R276-R283], we now do set.has(d.id) to filter features, where we used to do, d.id in completed.

The old way returns true for recently deleted features. But, the former will return false because this set has already filtered out the undefined entities.

What if we try the old filter?

@maxgrossman Thanks so much for hunting down this bug! Your deductions appears to be correct. I changed set to populate from the entity IDs in complete so that deleted IDs are accounted for.

You got it. I also see this...
image
image

It does not seem to be rendering what was the midpoint as a vertex if I undo a 3 node way to when I only drew the first segment.

Maybe a similar thing somewhere else? not sure...

Sorry, I think I hovered over the node in the first 2 pics I uploaded...
anyway, you can see that there's no vertex in the second image for node number 2.
I can see if that same window of lodash related commits has something to do with it.

It does not seem to be rendering what was the midpoint as a vertex if I undo a 3 node way to when I only drew the first segment.

I've also seen this, but I can't reproduce it now for some reason. It wouldn't surprise me if it's lodash-related as well. I think it's somewhat lower priority than the deletion issue was鈥攊t's really bad when iD tries to handle unloaded data.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tordans picture tordans  路  3Comments

1ec5 picture 1ec5  路  3Comments

jidanni picture jidanni  路  3Comments

Chaz6 picture Chaz6  路  3Comments

tmcw picture tmcw  路  3Comments