|
|
#31 |
|
Sigil Developer
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 9,838
Karma: 7500000
Join Date: Nov 2009
Device: many
|
Okay, I now feel better! I repeated the original test case on Sigil-2.7.6 and eventually I hit the exact same bug:
Code:
<spine>
<itemref idref="Section0001.xhtml"/>
<itemref idref="Section0003.xhtml"/>
<itemref idref="Section0004.xhtml"/>
<itemref idref="Section0005.xhtml"/>
<itemref idref="Section0006.xhtml"/>
<itemref idref="Section0007.xhtml"/>
<itemref idref="nav.xhtml" linear="no"/>
<itemref idref="Section0002.xhtml"/>
<itemref idref="Section0004.xhtml"/>
<itemref idref="Section0006.xhtml"/>
</spine>
Whew, I thought I was going crazy! The bug still exists in Sigil 2.7.6 but it is just harder to hit. It took me 5 total tries to see the bug. So the websites are correct, the bug is in Qt when trying to move non-contiguous selection of files, and it exists in Sigil 2.7.6 as well. It is just harder to trigger. So what changed is event order or timing. My sanity is now safe! And the push I just made to master is truly needed, and we still need a way to separate non-contiguous files to group them either at the top or the bottom keeping relative order. I will now start looking into that. Last edited by KevinH; 07-23-2026 at 10:13 AM. |
|
|
|
|
|
#32 |
|
Sigil Developer
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 9,838
Karma: 7500000
Join Date: Nov 2009
Device: many
|
I repeated the test to make sure and it took me over 10 times to trigger the bug using the original test case. So the bug in Sigil 2.7.6 is there (at least on a Mac), it is really just hard to trigger.
|
|
|
|
|
|
#33 |
|
Grand Sorcerer
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 29,623
Karma: 212299010
Join Date: Jan 2010
Device: Nexus 7, Kindle Fire HD
|
Good to know!
|
|
|
|
|
|
#34 |
|
Wizard
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 1,008
Karma: 3640080
Join Date: Jan 2017
Location: Poland
Device: Various
|
So I haven't gone crazy. After all, over the years I would have noticed an EPUB file with a corrupted spine section. This must be rare, but in version 2.8.1 it's "somehow" more common.
|
|
|
|
|
|
#35 |
|
Sigil Developer
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 9,838
Karma: 7500000
Join Date: Nov 2009
Device: many
|
It is and I have tracked down what makes this thing trigger more often is the one final call to Refresh() in BookBrowser after each call to UpdateHTMLReadingOrders in the OPFModel.cpp source code. If I remove that final call (which was a change since 2.7.6) I go back to rarely or if ever triggering the bug anymore.
So there is a race between between when drag and drop signals we need to update the html reading order and when we are in Refresh which of course always properly updates the html reading order too. This race only seem to happen after a non-consecutive move which has emitted the signal but is still working on the actual drag and drop. This does not happen with consecutive drag and drop. So we are missing further invocations of this routine because it took too long. I think with some careful changes, I might be able to even remove this conflict. Let me play around a bit with it. Last edited by KevinH; 07-23-2026 at 10:43 AM. |
|
|
|
|
|
#36 |
|
Sigil Developer
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 9,838
Karma: 7500000
Join Date: Nov 2009
Device: many
|
And I have learned something else. I now think the reports of Qt not handling non-contiguous drag and drop are all based on the speed of the routine people use to tell their code it needs to update based on an internal drag and drop.
In 2.8.0, and 2.8.1, this routine took longer as it did a full refresh (which is useful but ...) In 2.7.6 this routine did not call Refresh() and was much much faster. This made the race condition much much harder to hit. If with the added Refresh, I see that this routine only gets called once by the drag and drop code and therefore misses later invocations from the internal drag and drop code. Without that Refresh() call, it is faster and debugging now shows that the number of times this routine gets invoked is the number of non-consecutive selections used in the drag and drop code! So if there are 3 disjoint sections, we got called 3 times by the drag and drop. If there were four disjoint sections, we got called 4 times, and etc. So by changing the timing (how much work went on in that routine) to include a full refresh while technically correct, made us miss the additional invocations caused by non-consecutive selections in drag and drop. So my question is the following: A. Should I remove my latest change to master and replace it by removing the added call to Refresh so that this routine runs more quickly and thus greatly reduces the chance of errors happening (and I can try to speed up the code more just in case) and we just live with the potential of occasional duplication in the spine that will fix itself with the next change. (ie. act more like Sigil 2.7.6) OR B. Keep the current change in master but still remove the extra call to Refresh (as speed even in the contiguous case still matters) and add a much safer non-drag and drop way to order non-contiguous selections at the top or bottom of the Reading Order to make consecutive selection much easier. OR C. Try to devise a timer to catch the fact that additional signals from drag and drop were made and use it to trigger a last update call. This is doable but a bit invasive. We use a similar trick but in reverse with the Preview Update timer so that a additional manual editing before the timeout just grows the timeout. Effectively it would be like you running a single drag and drop once after a nonconsecutive drap and drop generates missed signals. This approach may have secondary issues. I am open to any of these approachs. The impact of the speed of the routine is the key here for preventing this duplication. The more responsive I can make it the less likely the error. Thoughts? Last edited by KevinH; 07-23-2026 at 10:54 AM. |
|
|
|
|
|
#37 |
|
Grand Sorcerer
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 29,623
Karma: 212299010
Join Date: Jan 2010
Device: Nexus 7, Kindle Fire HD
|
A. definitely sounds like the lesser of two evils here. But I'm still not crazy about leaving things up to chance.
|
|
|
|
|
|
#38 |
|
Bibliophagist
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 53,401
Karma: 181843001
Join Date: Jul 2010
Location: Vancouver
Device: Kobo Sage, Libra Colour, Lenovo M8 FHD, Paperwhite 4, Tolino epos
|
I was testing the update to prevent Sigil from dragging/dropping non-contiguous files and while that drag/drop is prevented, the select of the non-contiguous files seems to work until you start the drag at which point it looks as if trying to select other files is happening.
I reverted to the release compile of Sigil 2.8.1 and tried the non-contiguous drag/drop and in 20 different ePubs, a simple move of a single or contiguous file corrected the issue in the opf file. OTOH, I may be suffering from Friday afternoon syndrome a day early but I can't seem to spot the Edit Reading Order in the Tools dropdown menu. Ahh... not the Tools menu but Change Reading Order in the right click menu for a text file. ![]() As for the A/B suggested fix, I would go with A as being the easiest and least intrusive. Perhaps saving B or C for a later update. Last edited by DNSB; 07-23-2026 at 02:57 PM. |
|
|
|
|
|
#39 | |
|
Wizard
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 1,008
Karma: 3640080
Join Date: Jan 2017
Location: Poland
Device: Various
|
A
It's also a good idea to educate users: before saving a file, it's a good idea to check it with EpubCheck. Quote:
EDIT: Oh! This is my 1,000th post on the MR forum! Last edited by BeckyEbook; 07-23-2026 at 03:25 PM. |
|
|
|
|
|
|
#40 |
|
Sigil Developer
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 9,838
Karma: 7500000
Join Date: Nov 2009
Device: many
|
Well everybody chose A which makes my job quite easy. I will remove my last push to master and remove the use of Refresh at the end of the UpdateHTMLReadingOrders routine. I will also look for opportunities to see if I can speed it up any further.
I will work on that tomorrow. |
|
|
|
|
|
#41 |
|
Sigil Developer
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 9,838
Karma: 7500000
Join Date: Nov 2009
Device: many
|
okay, I have pushed both the revert of the last commit and removal of the added call to Refresh so we should be back to Sigil 2.7.6 behaviour here.
On the good news front, upcoming changes (already in my personal repo) speed up BookBrowser enough so that I am now *unable* to recreate any issues at all with non-contiguous drag and drops on those builds. And I tested them repeatedly. That said, the issue might still exist if an epub's spine contains thousands of entries and the user is dragging hundreds of non-contiguous selections at the same time but that is definitely not a common use case. I will look into somehow recording missed non-consecutive drag and drop signals in the now very rare case it happens and at least alerting the user or rerunning the Reading Order rebuild. But that is something for the future. So I now consider this issue "fixed". Thanks to "byword" for reporting it and providing a clear reproducible test case, and thanks for everyone who tested or supplied feedback. So hopefully we are now back to where we were in Sigil 2.7.6 and will soon be even better when the BookBrowser speedups make it into Sigil master. |
|
|
|
|
|
#42 |
|
Bibliophagist
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 53,401
Karma: 181843001
Join Date: Jul 2010
Location: Vancouver
Device: Kobo Sage, Libra Colour, Lenovo M8 FHD, Paperwhite 4, Tolino epos
|
For anyone interested in testing under Windows, you can download an updated Sigil-2.8.1-Windows-x64-Setup.exe from this link.
|
|
|
|
|
|
#43 |
|
Sigil Developer
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() Posts: 9,838
Karma: 7500000
Join Date: Nov 2009
Device: many
|
|
|
|
|
|
|
#44 |
|
Member
![]() Posts: 24
Karma: 10
Join Date: Apr 2022
Device: Onyx Leaf 3
|
I downloaded the source code uploaded to Git today, built it, and tested it.
OS: Windows 11 I created EPUB2 and EPUB3 files interchangeably and ran about 10 tests, and the duplicate spine reference issue did not occur a single time. The issue that intermittently appears in version 2.7.6 hasn't occurred for me yet, and if it only happens about 1 in 10 times, I can resolve it by manually editing the opf file. Thank you to everyone for your hard work! |
|
|
|
![]() |
|
Similar Threads
|
||||
| Thread | Thread Starter | Forum | Replies | Last Post |
| manifest and spine issue | larysa | Editor | 4 | 10-17-2021 12:07 PM |
| Pulling my hair out: reference not found and src path errors | meditateauthor | ePub | 12 | 08-30-2021 12:21 PM |
| Can a file have multiple semantic tags (epub:type, reference type="…")? | jcsalomon | ePub | 7 | 02-19-2018 11:40 AM |
| How do I add a file to the spine? | cager | Editor | 5 | 01-08-2015 02:22 PM |
| html file outside spine? | Ryn | ePub | 14 | 03-27-2012 04:31 AM |