Issue with manifest items being declared multiple times.

[byword]

It seems that manifest items are being duplicated when adding files using the newly added Drop Zone.

Steps to Reproduce:

1. Add a file via the Drop Zone.
2/ Add another file with the exact same name via the Drop Zone again.

When adding files this way, two manifest items are created: one using the filename ID and another using a random ID.

Example of duplicate manifest items generated by Sigil.

<item id="txt_69-2.png" href="Images/txt_69-2.png" media-type="image/png"/>
<item id="x9dbbd7d2-c149-4d38-9e87-21079c863550" href="Images/txt_69-2.png" media-type="image/png"/>

Note: For reference, I am using a custom build from the latest source code, which includes the fixes for the prettify issue and the CSS synchronization issue.

[BeckyEbook]

I always wonder how to add another file with the same name, because I don't know how to do it.

There must be some other factors at play here that I'm not aware of.

[KevinH]

I think this may be a duplicate of this bug:

https://github.com/Sigil-Ebook/Sigil/issues/871

That should already be fixed in master. Please see if your build contains this fix:

https://github.com/Sigil-Ebook/Sigil...a39b4b805cce1b

Please note: DropZone uses the AddExisting code.

I will try adding the same png twice? It should ask you if you want to overwrite the existing image. Just like BeckyEbook's image shows. If not, it is a different bug.

Previously this bug happened by pulling in two different xhtml files that both linked to the same png image but the change linked to above was tested and fixed that.

Interestingly, I could never recreate this issue on MacOS, only Windows and Linux.

[DiapDealer]

I'm getting the same results as the OP on Windows with the latest commits and following the steps provided (including using a PNG).

I always get the dialog asking if I want to overwrite the existing file or not. If I click "No," nothing is added (that seems correct to me). If I click "Yes," the existing png file gets overwritten (different pngs; same name to test), but a duplicate entry is added to the manifest with a generated uuid as the item id.

EDIT: I never tested the original bugfix by adding files individually. That issue only arose when adding a bunch of files at once that had links to the same file. In fact, that bug could be worked around by adding the css first, then the images, and then the xhtml. Perhaps we just shifted the issue to individually added files somehow?

[KevinH]

Argh! There is a special case needsToUpdateOPF var to handle just that case. It has gotten broken with the fix.

Originally Posted by DiapDealer

I'm getting the same results as the OP on Windows with the latest commits and following steps provided (including using a PNG).

I always get the dialog asking if I want to overwrite the existing file or not. If I click "No," nothing is added (that seems correct to me). If I click "Yes," the existing png file gets overwritten (different pngs; same name to test), but a duplicate entry is added to the manifest with a generated uuid as the item id.

EDIT: I never tested the original bugfix by adding files individually. That issue only arose when adding a bunch of files at once that had links to the same file. In fact, that bug could be worked around by adding the css first, then the images, and then the xhtml. Perhaps we just shifted the issue to individually added files somehow?

[KevinH]

Okay, the whole idea behind needsToUpdateOPF was broken. I have reverted the code in AddExisting to do a full delete of the image resource with immediate updating of the opf before re-adding it with its replacement.

Correct and slower is much better than sometimes incorrect and faster.

We can revisit this in the future by directly writing the new image data to its underlying file with no changes at all to the OPF to re-speed things up. But I want stability now, not more changes that may be broken in some way. There are just too many code paths in that one routine and too many special cases to handle. It should probably be completely rewritten at some point. But right now it should be fully working.

I have pushed this fix to master. So if you build your own, please update it.
(DiapDealer I used deploy so you should be able to grab a build too).

Thank you for your bug report!

[DiapDealer]

Yes. That seems to have taken care of it. I can't find my folder full of 1000 images to see how much it slowed back down, but I agree that slower and stable beats fast and buggy every time.

[DNSB]

I build the update and, for me, it seems a bit slower with my test ePub but slower beats broken.

[KevinH]

The slowdown should be minor unless all 1000 images are actually being replaced! Only the replacement of an image would slow things down the way it is now.

[DNSB]

Originally Posted by KevinH

The slowdown should be minor unless all 1000 images are actually being replaced! Only the replacement of an image would slow things down the way it is now.

Only 512 images but that is exactly what my test does. On my computer, I can tell that it is slower but not to the point that I'm being bothered by it.

[KevinH]

After our upcoming Sigil 2.8.1 release in a few weeks, I will try an alternative approach to handling replacements of images without needing to touch the opf at all, which should speed things up again. That code is just a bit too fragile to play with for a pure bug fix release.