Sigil-0.9.991 (1.0 Pre-release) - Page 11

DiapDealer · 11-16-2019, 09:25 AM

@KevinH: if I'm reading things right, there is no current html resource being passed to the select_text dialog (SelectTarget() in Dalogs/EditTOC.cpp), thus m_CurrentHTMLResource is null in Dialogs/SetHyperlinks.cpp. This causes the crash in SetHyperlinks::SetList() somewhere in the foreach loop.. I can test for a null m_CurrentHTMLResource and prevent the crash, but then no targets from the epub are added to the resulting select dialog.

In other words: I think Dialogs/SetHyperlinks.cpp doesn't know what the current html resource IS, but it's still trying to use it.

KevinH · 11-16-2019, 09:53 AM

probably some I broke somehow. I will try to recreate this and get a backtrace? Does this happen on linux as well?

KevinH · 11-16-2019, 10:11 AM

Actually passing NULL is not from me! I checked the history on that file and it has been like that from before 2015.

How on earth has this bug not existed since the beginning of Sigil?

Doitsu · 11-16-2019, 10:33 AM

Quote:

Originally Posted by KevinH

Does this happen on linux as well?

Yes. I got the following message:

Code:

Warning: Empty filename passed to function

DiapDealer · 11-16-2019, 10:50 AM

Quote:

Originally Posted by KevinH

Actually passing NULL is not from me! I checked the history on that file and it has been like that from before 2015.

How on earth has this bug not existed since the beginning of Sigil?

Wow! That is odd. I guess I don't know for a fact that the bug HASN'T been around that long. Most people don't manually edit the toc. And if they do, they're probably more likely to do so by editing the NCX (and now NAV) directly. The few times I've used the EditToC dialog, I manually entered the targets (double-click the treeview entry) instead of looking them up via Select Target.

Doitsu · 11-16-2019, 10:55 AM

Quote:

Originally Posted by DiapDealer

Wow! That is odd. I guess I don't know for a fact that the bug HASN'T been around that long.

The bug wasn't present in Sigil 0.9.14.

DiapDealer · 11-16-2019, 11:04 AM

That's about when we left Qt5.11 behind (0.9.15 was the prelease for QtWebEngine/Qt5.12). I'm guessing it was a change in behavior in Qt5.12.

Either way: m_CurrentHTMLResource in Dialogs/SelectHyperlink.cpp has been NULL since time began and the feature still worked. So it (m_CurrentHTMLResource) seemed to have no bearing on the conditionals it was being used in back when it WAS still working. That's crazy.

Basically, the list of resources being passed to Dialogs/SelectHyperlink.cpp was used (which included the current resource as well), while the null resource seemed to be ignored altogether.

The gist of the logic to my eyes, was to add the targets from the currently selected item first, and then the rest of targets from the book. So all the targets were still present, there was simply no prioritizing of the list going on.

KevinH · 11-16-2019, 11:05 AM

EditTOC and InsertHyperLinks code are really at odds with each other. In MainWindow, The Hyperlinks code is used to create a link from one htmlresource to another resource and it works just fine. It is designed to return links relative to the starting location.

The TOC entries no longer uses relative hrefs and instead use bookpaths that are independent of starting location to indicate a target.

I am going to have to make some changes to both pieces to get this to work.

This is going to be a bit of a pain!

Kevin

DiapDealer · 11-16-2019, 11:26 AM

If that's the case, we should be able to skip attempting to add the currently selected resource's targets first, and then ditch the tests designed to make sure the currently selected resource's targets haven't been already processed. That should leave us with the functionality we've always had--minus the new crashing behavior.

EDIT: Never mind. I didn't realize SelectHyperlinks was being used elsewhere.

KevinH · 11-16-2019, 11:50 AM

This should now be fixed in master. I had to synchronize the usage of SelectHyperLinks from two places and post convert to bookpaths for EditToc.

Hopefully we are now good to go.

Doitsu · 11-16-2019, 01:13 PM

Quote:

Originally Posted by KevinH

This should now be fixed in master. I had to synchronize the usage of SelectHyperLinks from two places and post convert to bookpaths for EditToc.

I've tested 0.9.991-32 on my Windows 10 machine with standard and non-standard epub2 and epub3 books and didn't get any segfaults when I selected TOC targets.

D.

DiapDealer · 11-16-2019, 01:21 PM

Quote:

Originally Posted by KevinH

This should now be fixed in master. I had to synchronize the usage of SelectHyperLinks from two places and post convert to bookpaths for EditToc.

Hopefully we are now good to go.

The default value in SelectHyperlinks seems to includes some extra prefixing (OEBPS on an epub with the Sigil standard structure) that can show an incorrect href in EditTOC if the OK button is hit. Look likes it's entirely cosmetic though. The correct link/href seems to be added to the ncx regardless.

snarkophilus · 11-16-2019, 03:36 PM

Quote:

Originally Posted by KevinH

This should now be fixed in master. I had to synchronize the usage of SelectHyperLinks from two places and post convert to bookpaths for EditToc.

Hopefully we are now good to go.

Looks good to me with a bit of playing around, editing and inserting TOC entries. Thanks again guys!

Quote:

Originally Posted by DiapDealer

Wow! That is odd. I guess I don't know for a fact that the bug HASN'T been around that long. Most people don't manually edit the toc. And if they do, they're probably more likely to do so by editing the NCX (and now NAV) directly. The few times I've used the EditToC dialog, I manually entered the targets (double-click the treeview entry) instead of looking them up via Select Target.

I use the TOC editor reasonably often, mostly because it takes care of the play order numbering especially if I delete an early entry in a long TOC. If I want to change the identing (eg chapters of parts) sometimes it's easier to do this in the TOC editor too because it re-indents the NCX nicely. For just a content tweak without changing the order I'll update the NCX directly.

I think I've added a TOC entry recently, since the Qt5.12 switch, but I can't be totally sure.

KevinH · 11-16-2019, 03:37 PM

Yes, EditToc now uses full bookpaths. I will have to modify the href on the way into SelectHyperlinks just like I had to convert to bookpaths on the way in.

snarkophilus · 11-16-2019, 03:42 PM

Quote:

Originally Posted by snarkophilus

Well ... if I rename another file within that epub, then I get the attached message. Sigil keeps working, other that the pop-up everything else works. I expect that's related to updating/changing the contents.opf?

Now that I've interrupted the flow of this thread with a different issue, any thoughts on the popup I get with my funny unsupported media type popup in this message https://www.mobileread.com/forums/sh...71#post3916771?

11-16-2019, 09:25 AM	#151
DiapDealer Grand Sorcerer Posts: 28,525 Karma: 204127028 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	@KevinH: if I'm reading things right, there is no current html resource being passed to the select_text dialog (SelectTarget() in Dalogs/EditTOC.cpp), thus m_CurrentHTMLResource is null in Dialogs/SetHyperlinks.cpp. This causes the crash in SetHyperlinks::SetList() somewhere in the foreach loop.. I can test for a null m_CurrentHTMLResource and prevent the crash, but then no targets from the epub are added to the resulting select dialog. In other words: I think Dialogs/SetHyperlinks.cpp doesn't know what the current html resource IS, but it's still trying to use it. Last edited by DiapDealer; 11-16-2019 at 09:30 AM.

11-16-2019, 11:04 AM	#157
DiapDealer Grand Sorcerer Posts: 28,525 Karma: 204127028 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	That's about when we left Qt5.11 behind (0.9.15 was the prelease for QtWebEngine/Qt5.12). I'm guessing it was a change in behavior in Qt5.12. Either way: m_CurrentHTMLResource in Dialogs/SelectHyperlink.cpp has been NULL since time began and the feature still worked. So it (m_CurrentHTMLResource) seemed to have no bearing on the conditionals it was being used in back when it WAS still working. That's crazy. Basically, the list of resources being passed to Dialogs/SelectHyperlink.cpp was used (which included the current resource as well), while the null resource seemed to be ignored altogether. The gist of the logic to my eyes, was to add the targets from the currently selected item first, and then the rest of targets from the book. So all the targets were still present, there was simply no prioritizing of the list going on. Last edited by DiapDealer; 11-16-2019 at 11:20 AM.

11-16-2019, 11:26 AM	#159
DiapDealer Grand Sorcerer Posts: 28,525 Karma: 204127028 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	If that's the case, we should be able to skip attempting to add the currently selected resource's targets first, and then ditch the tests designed to make sure the currently selected resource's targets haven't been already processed. That should leave us with the functionality we've always had--minus the new crashing behavior. EDIT: Never mind. I didn't realize SelectHyperlinks was being used elsewhere. Last edited by DiapDealer; 11-16-2019 at 11:28 AM.

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
5.8.11 Pre=release	knc1	Kindle Developer's Corner	21	04-17-2018 08:42 PM
Overdrive - Search for Pre-Release	nynaevelan	General Discussions	11	11-24-2013 02:27 PM
Where are the pre-release purchase buttons?	Sydney's Mom	General Discussions	8	09-06-2012 10:57 PM
KF Android 4.0 Pre-Release Version	robertc88	Kindle Fire	22	01-22-2012 07:24 PM
PDF Viewer 0.3.0 pre-release	pruss	Android Devices	62	11-22-2011 11:18 AM

11-16-2019, 09:53 AM	#152
KevinH Sigil Developer Posts: 8,704 Karma: 5703586 Join Date: Nov 2009 Device: many	probably some I broke somehow. I will try to recreate this and get a backtrace? Does this happen on linux as well?

11-16-2019, 10:11 AM	#153
KevinH Sigil Developer Posts: 8,704 Karma: 5703586 Join Date: Nov 2009 Device: many	Actually passing NULL is not from me! I checked the history on that file and it has been like that from before 2015. How on earth has this bug not existed since the beginning of Sigil?

11-16-2019, 11:05 AM	#158
KevinH Sigil Developer Posts: 8,704 Karma: 5703586 Join Date: Nov 2009 Device: many	EditTOC and InsertHyperLinks code are really at odds with each other. In MainWindow, The Hyperlinks code is used to create a link from one htmlresource to another resource and it works just fine. It is designed to return links relative to the starting location. The TOC entries no longer uses relative hrefs and instead use bookpaths that are independent of starting location to indicate a target. I am going to have to make some changes to both pieces to get this to work. This is going to be a bit of a pain! Kevin

11-16-2019, 11:50 AM	#160
KevinH Sigil Developer Posts: 8,704 Karma: 5703586 Join Date: Nov 2009 Device: many	This should now be fixed in master. I had to synchronize the usage of SelectHyperLinks from two places and post convert to bookpaths for EditToc. Hopefully we are now good to go.

11-16-2019, 03:37 PM	#164
KevinH Sigil Developer Posts: 8,704 Karma: 5703586 Join Date: Nov 2009 Device: many	Yes, EditToc now uses full bookpaths. I will have to modify the href on the way into SelectHyperlinks just like I had to convert to bookpaths on the way in.

Advert

Advert