Add Cover/InsertImageSVG - another bug?

odamizu · 11-14-2019, 06:01 PM

I may have discovered another bug connected to Add Cover and InsertImageSVG.

use Tools > Add Cover to add a cover
in the new file, make an html error — e.g., delete an opening <div> or <body> or <html>, etc.
pink error box does not appear!

However, if I make an html error in any other file, the pink error box appears.

This is happening with any file added with Add Cover or InsertImageSVG. It is occurring with new blank epubs and existing epubs that already have files in it.

This is occurring on 0.9.991, 0.9.991-build 208 and 0.9.991-build 211.

It does not occur with 0.9.18.

I am testing on macOS High Sierra.

Let me know what I can do to help

DiapDealer · 11-14-2019, 06:26 PM

Hmmm....

On Linux anyway (very latest commits):

1) open a new empty EPUB2 (standard default structure)
2) add an image to the epub
3) use Tools->Add Cover to create a cover page with that image
4) see cover.xhtml rendering in Preview
5) delete the opening body tag's closing bracket
6) instant pink error in Preview

EDIT: Nevermind. I missed the part about deleting the whole opening tag. You're right. That's weird!

Even after closing and reopening cover.xhtml, Preview will happily render the page with no body tags whatsoever. Delete the body tags from any other file (or delete just a portion of one the tags in cover.xhtml) and you get a pink box.

Inspector show the body tags as being present on the newly added cover.xhtml (with the body tags deleted), but it shows them as missing on any other file (with the body tags deleted).

KevinH · 11-14-2019, 06:35 PM

Could be html vs xhtml rules? Perhaps we are using setHTML to view things instead of setContent with the xhtml mimetype.

I will take a peak. Either way if you sanity check the xhtml, it should report the missing open body tag.

Kevin

DiapDealer · 11-14-2019, 06:41 PM

I can't get over the fact that even saving the epub and reopening it isn't enough to make that one file start behaving normally. You can delete whole tags in it willy nilly and Preview will correct it behind the scenes. But you can't do that with any other xhtml files in the epub.

Quote:

Originally Posted by KevinH

Either way if you sanity check the xhtml, it should report the missing open body tag.

I can comfirm this is true

KevinH · 11-14-2019, 06:49 PM

Does it have the proper doctype set?
Does it have the xml declaration as the first line?

Could it be that when parsing html with svg they relax the rules?

I checked, under html5 the body start and end tags are optional. Of course they are required for xhtml but something is convincing the QWebEngine to parse it as html and not xhtml.

The Preview code is always loaded via SetContent with the xhtml media type, so something different about that file is causing it to be parsed under html rules.

Hmmmm

KevinH · 11-14-2019, 06:58 PM

I bet it is the "standalone="no" in the xml header in Book.cpp used in HTML_COVER_SOURCE and in HTML5_COVER_SOURCE.

I bet if you remove them then the normal html dtd rules would apply.

This is just a wag, as I am away from my dev machine.

DiapDealer · 11-14-2019, 07:21 PM

I rebuilt with standalone="no" removed and it didn't seem to make any difference. So far as I can see, there's no difference in the code from the head tag up in the cover.xhtml file compared to any other xhtml file that's behaving normally. I'll tear the epub apart and examine it outside of Sigil. There HAS to be something there to see for the issue to survive a close/reopen.

DiapDealer · 11-14-2019, 07:51 PM

It's the svg code itself! Remove the svg tags and everything in between and the file starts behaving normally. Which leads me to believe this has everything to do the fixup for full-screen images that I suggested. Something with the "100vh" and "100vw" we inject in place of 100% for the width/height attributes is tripping up QWebEngineView for some reason.

Changing the height and width parameters of the svg from 100% to 99% will stop the replacement and allow the QWebEngineView to treat the file like xhtml again.

Weird. We'll clearly have to rethink that.

KevinH · 11-14-2019, 07:57 PM

Yes that is very weird! What should we do about it? Perhaps just stop replacing with “vh” and “vw”?

KevinH · 11-14-2019, 07:58 PM

Let me check the code in PreviewWindow to make sure I did not mess it up somehow.

DiapDealer · 11-14-2019, 08:02 PM

I suspect that only changing the height attribute to vh will suffice to make Preview behave the way everyone has come to expect full-screen svg-wrapped images to behave without triggering the weirdness.

I'll test that theory with a version of Sigil that doesn't make the substitution and report back.

DiapDealer · 11-14-2019, 08:23 PM

Yes. After some testing: the test to check that both height and width are 100% before making the substitution is fine, but changing only the height to 100vh allows the image to display the way everyone has come to expect svg-wrapped cover images to display. Without all the weirdness.

KevinH · 11-14-2019, 08:34 PM

Very strange. I will still double check the code that makes that swap and modify it to set only 100vh and to make sure I am not modifying the doctype or xml header in some strange way.

DiapDealer · 11-14-2019, 08:41 PM

Yes. There must be something wrong with the way we're inserting that change on the fly. Because manually using those same attributes in a version of Sigil that doesn't try to fixup the svg code works just fine. But if I use the new fixup routine to try and change one or both of the attributes automatically, the weirdness happens.

KevinH · 11-14-2019, 10:05 PM

I am being stupid, here. I used gumbo to change the code on the fly that is given to Preview (see the end of PreviewWindow.cpp). And gumbo will happily mend any and all errors! So even though CodeView shows the error (missing body), gumbo will repair it if and only if it changes the code to use vh and vw.

So the code being displayed by Preview will be different than the code shown in CodeView. To see this, if we load a page without a body start tag that gets the 100% changed to 100vh and 100vw and we fire up the Inspector and look at the code, we will see the body node was happily added back in before being loaded into Preview.

So the only way around this is to use gumbo just to test for whether to make the change or not (it is a complex set of conditions) and then use regular expressions to make the actual change.

I will try to code that up tomorrow.

11-14-2019, 06:01 PM	#1
odamizu just an egg Posts: 1,809 Karma: 6759036 Join Date: Mar 2015 Device: Kindle, iOS	Add Cover/InsertImageSVG - another bug? I may have discovered another bug connected to Add Cover and InsertImageSVG. use Tools > Add Cover to add a cover in the new file, make an html error — e.g., delete an opening <div> or <body> or <html>, etc. pink error box does not appear! However, if I make an html error in any other file, the pink error box appears. This is happening with any file added with Add Cover or InsertImageSVG. It is occurring with new blank epubs and existing epubs that already have files in it. This is occurring on 0.9.991, 0.9.991-build 208 and 0.9.991-build 211. It does not occur with 0.9.18. I am testing on macOS High Sierra. Let me know what I can do to help

11-14-2019, 06:26 PM	#2
DiapDealer Grand Sorcerer Posts: 28,471 Karma: 203853430 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	Hmmm.... On Linux anyway (very latest commits): 1) open a new empty EPUB2 (standard default structure) 2) add an image to the epub 3) use Tools->Add Cover to create a cover page with that image 4) see cover.xhtml rendering in Preview 5) delete the opening body tag's closing bracket 6) instant pink error in Preview EDIT: Nevermind. I missed the part about deleting the whole opening tag. You're right. That's weird! Even after closing and reopening cover.xhtml, Preview will happily render the page with no body tags whatsoever. Delete the body tags from any other file (or delete just a portion of one the tags in cover.xhtml) and you get a pink box. Inspector show the body tags as being present on the newly added cover.xhtml (with the body tags deleted), but it shows them as missing on any other file (with the body tags deleted). Last edited by DiapDealer; 11-14-2019 at 06:36 PM.

11-14-2019, 07:21 PM	#7
DiapDealer Grand Sorcerer Posts: 28,471 Karma: 203853430 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	I rebuilt with standalone="no" removed and it didn't seem to make any difference. So far as I can see, there's no difference in the code from the head tag up in the cover.xhtml file compared to any other xhtml file that's behaving normally. I'll tear the epub apart and examine it outside of Sigil. There HAS to be something there to see for the issue to survive a close/reopen. Last edited by DiapDealer; 11-14-2019 at 07:23 PM.

11-14-2019, 07:51 PM	#8
DiapDealer Grand Sorcerer Posts: 28,471 Karma: 203853430 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	It's the svg code itself! Remove the svg tags and everything in between and the file starts behaving normally. Which leads me to believe this has everything to do the fixup for full-screen images that I suggested. Something with the "100vh" and "100vw" we inject in place of 100% for the width/height attributes is tripping up QWebEngineView for some reason. Changing the height and width parameters of the svg from 100% to 99% will stop the replacement and allow the QWebEngineView to treat the file like xhtml again. Weird. We'll clearly have to rethink that. Last edited by DiapDealer; 11-14-2019 at 07:56 PM.

11-14-2019, 08:41 PM	#14
DiapDealer Grand Sorcerer Posts: 28,471 Karma: 203853430 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	Yes. There must be something wrong with the way we're inserting that change on the fly. Because manually using those same attributes in a version of Sigil that doesn't try to fixup the svg code works just fine. But if I use the new fixup routine to try and change one or both of the attributes automatically, the weirdness happens. Last edited by DiapDealer; 11-14-2019 at 08:47 PM.

11-14-2019, 06:35 PM	#3
KevinH Sigil Developer Posts: 8,558 Karma: 5703586 Join Date: Nov 2009 Device: many	Could be html vs xhtml rules? Perhaps we are using setHTML to view things instead of setContent with the xhtml mimetype. I will take a peak. Either way if you sanity check the xhtml, it should report the missing open body tag. Kevin

11-14-2019, 06:49 PM	#5
KevinH Sigil Developer Posts: 8,558 Karma: 5703586 Join Date: Nov 2009 Device: many	Does it have the proper doctype set? Does it have the xml declaration as the first line? Could it be that when parsing html with svg they relax the rules? I checked, under html5 the body start and end tags are optional. Of course they are required for xhtml but something is convincing the QWebEngine to parse it as html and not xhtml. The Preview code is always loaded via SetContent with the xhtml media type, so something different about that file is causing it to be parsed under html rules. Hmmmm

11-14-2019, 06:58 PM	#6
KevinH Sigil Developer Posts: 8,558 Karma: 5703586 Join Date: Nov 2009 Device: many	I bet it is the "standalone="no" in the xml header in Book.cpp used in HTML_COVER_SOURCE and in HTML5_COVER_SOURCE. I bet if you remove them then the normal html dtd rules would apply. This is just a wag, as I am away from my dev machine.

11-14-2019, 07:57 PM	#9
KevinH Sigil Developer Posts: 8,558 Karma: 5703586 Join Date: Nov 2009 Device: many	Yes that is very weird! What should we do about it? Perhaps just stop replacing with “vh” and “vw”?

11-14-2019, 07:58 PM	#10
KevinH Sigil Developer Posts: 8,558 Karma: 5703586 Join Date: Nov 2009 Device: many	Let me check the code in PreviewWindow to make sure I did not mess it up somehow.

11-14-2019, 08:02 PM	#11
DiapDealer Grand Sorcerer Posts: 28,471 Karma: 203853430 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	I suspect that only changing the height attribute to vh will suffice to make Preview behave the way everyone has come to expect full-screen svg-wrapped images to behave without triggering the weirdness. I'll test that theory with a version of Sigil that doesn't make the substitution and report back.

11-14-2019, 08:23 PM	#12
DiapDealer Grand Sorcerer Posts: 28,471 Karma: 203853430 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	Yes. After some testing: the test to check that both height and width are 100% before making the substitution is fine, but changing only the height to 100vh allows the image to display the way everyone has come to expect svg-wrapped cover images to display. Without all the weirdness.

11-14-2019, 08:34 PM	#13
KevinH Sigil Developer Posts: 8,558 Karma: 5703586 Join Date: Nov 2009 Device: many	Very strange. I will still double check the code that makes that swap and modify it to set only 100vh and to make sure I am not modifying the doctype or xml header in some strange way.

11-14-2019, 10:05 PM	#15
KevinH Sigil Developer Posts: 8,558 Karma: 5703586 Join Date: Nov 2009 Device: many	I am being stupid, here. I used gumbo to change the code on the fly that is given to Preview (see the end of PreviewWindow.cpp). And gumbo will happily mend any and all errors! So even though CodeView shows the error (missing body), gumbo will repair it if and only if it changes the code to use vh and vw. So the code being displayed by Preview will be different than the code shown in CodeView. To see this, if we load a page without a body start tag that gets the 100% changed to 100vh and 100vw and we fire up the Inspector and look at the code, we will see the body node was happily added back in before being loaded into Preview. So the only way around this is to use gumbo just to test for whether to make the change or not (it is a complex set of conditions) and then use regular expressions to make the actual change. I will try to code that up tomorrow.

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
Add a cover to 'auto add' conversions	skb	Conversion	4	07-14-2019 12:26 AM
Impossible to add a font to an azw3. Is it a bug?	Ruskie_it	Editor	4	07-03-2017 07:02 AM
Wrong-cover bug workaround?	rwhe	Kobo Reader	31	02-01-2013 05:33 PM
Bug? Error when attempting to add PDF in 0.8.2	SilentSeven	Calibre	2	05-25-2011 01:36 PM
Cover Browser (related) bug (0.7.3)	Stinger	Calibre	5	06-19-2010 07:30 PM

Advert

Advert