[GUI Plugin] Modify ePub - Page 13

jackie_w · 07-01-2012, 07:52 AM

Quote:

Originally Posted by ldolse

it's been a long time since I messed with this so I'd have to review the code in detail to answer this question fully. I don't think you'd get double margins from the plugin as it modifies both page and body margins - i.e. margins in body should be removed and everything would be in @page. It does sound like you're saying there may be an edge case if you run the plugin and then follow that by running a Calibre conversion. Calibre might insert the margins in the body tag whilst leaving @page alone, not sure of this, but since the whole point of the plugin is to avoid a Calibre conversion I'm not sure how critical it is. I sort of doubt this is an issue anyway as it could potentially happen with any number of retail ePubs as well and I haven't heard of Calibre doing that.]

I think the problem arises when the body tags in the html files are <body class="somename"> rather than simple <body>

and the non-zero body L/R margins are contained in the css file under .somename {margin...} rather than body{margin...}

Unfortunately, this would include all epubs which have already been calibre-converted plus some retail epubs.

Also, I know that a recent calibre change (much-appreciated, BTW) now puts any @page (and @font-face) in a 2nd page_styles.css file rather than in every single html file, but the current plugin does not remove @page from the htmls as part of this option. But maybe this was outside the scope.

JSWolf · 07-01-2012, 07:54 PM

Quote:

Originally Posted by ldolse

Calibre's preferences are hard-coded to pts.

If you want no margins in @page or body, then set your calibre preferences under 'page setup' in common conversion options to have all the margins zero - this will cause the plugin to work exactly as the initial implementation I posted did - any margins in @page or body will simply be deleted.

Otherwise, the plugin will delete all existing margins in body or @page, and then replaces them with the pt based margins specified in Calbre's preferences. It only puts the new margin settings in @page, so all margins set on body are always removed.

Capnm is correct in the extremely rare case that the epub css has multiple @page statements, some using 'ID', they will all be re-written, but any using an ID will just have them removed. I'm actually inclined to think that @page statements with an ID is invalid css anyway, but we'll see if someone else can provide some further enlightenment on that point. @page statements using a psuedo selector like :first would actually be ignored.

@JSWolf, I chose to put them in @page because this is the advice I've read from epub guru's like pdurrant, Kovid, and Jelby. I believe @page is more focused on device page boundaries vs. the entire text, which is what body will do. I would have liked to be able to research it more, but try googling @page - big fail. Removing the xpgt file entirely is a bit outside of the scope of this plugin, as it's focused on non/minimally-destructive changes. As Jelby has pointed out in the epub sub-forum, there are some potentially good uses for it, such as detecting whether ADE is the renderer and creating some work-around css styles that only it will render.

Edit - one last point I didn't see - if you don't like Calibre specifying top and bottom margins, simply set those to zero under page setup. That should eliminate them during conversions and with this plugin.

The idea was to REMOVE @page and not add one. So if there is an @page in the CSS, it gets dumped and if there isn't, it doesn't get added. The problem is you state is that you add @page if there isn't one and remove it if there is one. That's not going to work. We don't know if there is or isn't one. I just want it gone. Now if I run Modify ePub again, the @page is not touched. It should be gone. If you want, make an option to add/modify @page. But don't just add it in. I want to get rid of it. So please make an option to dump @page. As it is now, the @page/body option is a major (serious) bug that needs to go or be fixed. As it is written, the option is to rewrite @page/body and if there is none, adding it is incorrect based on the what the option is said to do. You cannot rewrite something that does not exist. Adding it if it's not there is not rewriting it. My need is to remove @page where it is be it in some CSS or XML. I'm not needing the body style modified as I have to fix that by hand anyway.

ldolse · 07-02-2012, 05:03 AM

Quote:

Originally Posted by jackie_w

I think the problem arises when the body tags in the html files are <body class="somename"> rather than simple <body>

and the non-zero body L/R margins are contained in the css file under .somename {margin...} rather than body{margin...}

Unfortunately, this would include all epubs which have already been calibre-converted plus some retail epubs.

I acknowledge what you're saying here, and I saw that I discussed this with capnm during the inital development and decided not to go there (can't recall all the details). I also don't recall having any problems relating to this with Calibre generated ePubs when I was doing the testing. Anyway I can put a bit of thought into handling it - since the code checks each flow anyway it wouldn't be too difficult to check what class is assigned to body. This stuff is a slippery slope though as we decided a long time ago to only do basic string manipulation and not run the css/xhtml through a proper parser which could make unforeseen modifications to the code.

Quote:

Originally Posted by jackie_w

Also, I know that a recent calibre change (much-appreciated, BTW) now puts any @page (and @font-face) in a 2nd page_styles.css file rather than in every single html file, but the current plugin does not remove @page from the htmls as part of this option. But maybe this was outside the scope.

Yeah, I would say that detecting that it's been converted by two different versions of Calibre with two different behaviors in this area, then cleaning up after the older version of Calibre is a bit out of scope

I don't think multiple @page statements are cumulative, so this probably isn't critical.

Quote:

Originally Posted by JSWolf

The idea was to REMOVE @page and not add one. So if there is an @page in the CSS, it gets dumped and if there isn't, it doesn't get added. The problem is you state is that you add @page if there isn't one and remove it if there is one. That's not going to work. We don't know if there is or isn't one. I just want it gone. Now if I run Modify ePub again, the @page is not touched. It should be gone. If you want, make an option to add/modify @page. But don't just add it in. I want to get rid of it. So please make an option to dump @page. As it is now, the @page/body option is a major (serious) bug that needs to go or be fixed. As it is written, the option is to rewrite @page/body and if there is none, adding it is incorrect based on the what the option is said to do. You cannot rewrite something that does not exist. Adding it if it's not there is not rewriting it. My need is to remove @page where it is be it in some CSS or XML. I'm not needing the body style modified as I have to fix that by hand anyway.

I'm not sure where you got the idea that the purpose of this feature is to remove margins - the feature is called 'Rewrite Margins', not 'delete all @page margins'. That said I'm pretty sure you get what you're asking for when you set all of Calibre's margins to zero.

The plugin does know if there is already an @page or not and rewrites it appropriately, aside from moving left/right from body to @page as discussed previously - I understand why Kovid uses body now, but modifying the code to match Calibre would be non-trivial at this point, and what I've done works fine on ADE renderers. This is the first time it's been discussed in over a year, so it doesn't sound particularly critical to me.

If you're actually describing a specific use case where the plugin is failing you (but falls within the scope of the feature), PM me a link to a book and steps to reproduce what's bugging you.

Edit - to be quite clear - the feature as it's designed is meant to work with the matching check in quality check, and the point is to make sure that all ePub books have margins matching your Calibre preferences. If the book doesn't have margins to start with but you have them specified in your Calibre preferences then the plugin will insert them in order to match Calibre. Top & bottom margins are only supported in @page. Like I said before, if you don't want any margins at all then set your Calibre preferences to zero - if setting them to zero isn't working let me know (Will test Jackie's scenario as mentioned above).

JSWolf · 07-02-2012, 09:23 AM

Quote:

Originally Posted by ldolse

I'm not sure where you got the idea that the purpose of this feature is to remove margins - the feature is called 'Rewrite Margins', not 'delete all @page margins'. That said I'm pretty sure you get what you're asking for when you set all of Calibre's margins to zero.

The plugin does know if there is already an @page or not and rewrites it appropriately, aside from moving left/right from body to @page as discussed previously - I understand why Kovid uses body now, but modifying the code to match Calibre would be non-trivial at this point, and what I've done works fine on ADE renderers. This is the first time it's been discussed in over a year, so it doesn't sound particularly critical to me.

If you're actually describing a specific use case where the plugin is failing you (but falls within the scope of the feature), PM me a link to a book and steps to reproduce what's bugging you.

Edit - to be quite clear - the feature as it's designed is meant to work with the matching check in quality check, and the point is to make sure that all ePub books have margins matching your Calibre preferences. If the book doesn't have margins to start with but you have them specified in your Calibre preferences then the plugin will insert them in order to match Calibre. Top & bottom margins are only supported in @page. Like I said before, if you don't want any margins at all then set your Calibre preferences to zero - if setting them to zero isn't working let me know (Will test Jackie's scenario as mentioned above).

The name of the option is rewrite, not add in if missing. So if there is no @page, then the plugin should not add one as there's nothing to rewrite. You've got it wrong by adding in an @page wen one doesn't exist. That's not rewriting. That's adding. Not the same at all.

As for the bug, margins are set to no margins and yet the we still get

Code:

@page {
    
}

There was no @page in the first place so there should not be an @page at all. And I do think with margins set to none, if there is an @page, it should be removed when no margins are specified.

DiapDealer · 07-02-2012, 09:29 AM

Quote:

I did request to have @page removed. I don't want it changed to no margins. I want it gone. There's a big difference between adding when you say rewriting and adding when it's supposed to be removed.

There's also a pretty big difference between "what Jon wishes it would do" and "what the people who wrote it intended it to do."

JSWolf · 07-02-2012, 09:31 AM

Quote:

Originally Posted by DiapDealer

I...

Please change your message as what you quoted isn't valid as you'll see by my edit.

DiapDealer · 07-02-2012, 12:07 PM

Quote:

Originally Posted by JSWolf

Please change your message as what you quoted isn't valid as you'll see by my edit.

I'll take it under consideration, but I'm not sure I can be bothered at this point.

ldolse · 07-02-2012, 01:02 PM

@JSWolf, As I said before, the plugin code inserts @page by design - this is the only way to insert top and bottom page margins to match Calibre's preferences - this is the only way you can use modify ePub to fix the corresponding check in quality check. Inserting @page in general is not a bug and it's not going to change.

Aside from offending your sensibilities, an empty @page declaration is going to behave exactly the same as not inserting @page, but I'm ok with calling this case an issue, albeit a low priority one since it won't affect how the book renders. Whenever I get a chance to dig back into the plugin to investigate Jackie's case I'll look into not inserting empty @page statements when all margins are zero.

JSWolf · 07-02-2012, 03:08 PM

Quote:

Originally Posted by ldolse

@JSWolf, As I said before, the plugin code inserts @page by design - this is the only way to insert top and bottom page margins to match Calibre's preferences - this is the only way you can use modify ePub to fix the corresponding check in quality check. Inserting @page in general is not a bug and it's not going to change.

Aside from offending your sensibilities, an empty @page declaration is going to behave exactly the same as not inserting @page, but I'm ok with calling this case an issue, albeit a low priority one since it won't affect how the book renders. Whenever I get a chance to dig back into the plugin to investigate Jackie's case I'll look into not inserting empty @page statements when all margins are zero.

Could you also add in an option to delete @page? Also, if both options are checked, deleting @page should take higher priority.

ldolse · 07-02-2012, 08:06 PM

Quote:

Originally Posted by JSWolf

Could you also add in an option to delete @page? Also, if both options are checked, deleting @page should take higher priority.

I don't see this happening, for a variety of reasons:

You have completely failed to explain any useful reason to do this other than "I hate @page declarations and I have a personal vendetta against them" (I'm paraphrasing). They have a completely valid use for readers which support them, other readers will ignore them, and this feature lets you control the declaration.
It's more complexity to the plugin from a UI perspective for little to no benefit.
As you point out, it would conflict with the rewrite action, and it creates a situation that could prevent a user from ever 'fixing' the matching check in Quality Check.
If I fix the issue you've already highlighted you'll get what you want by setting margins to zero.
Kiwidude is the ultimate owner of the plugin, I can continue to tweak the functions I've added with his approval in the past, but I'm not going to stick in new functions without his sign-off.

JSWolf · 07-02-2012, 09:12 PM

Kiwidude, can we have an option to delete @page please?

44reader · 07-03-2012, 07:16 AM

I've accidentally run "smarten punctuation" on all my files.

I'm at the box where it says "Modify ePub modified x file(s). Proceed with updating your library?'

I'm really hoping that clicking No will leave my epubs unchanged. Is that the case?

This leads me to be optimistic:

Quote:

Originally Posted by kiwidude

... just click "Yes" and it will replace the ePub in your library with the epub it has now modified. ...

It's still sitting like that now awaiting a kind forum user to help me out.

Thank you!

jackie_w · 07-03-2012, 07:39 AM

Quote:

Originally Posted by 44reader

I've accidentally run "smarten punctuation" on all my files.

I'm at the box where it says "Modify ePub modified x file(s). Proceed with updating your library?'

I'm really hoping that clicking No will leave my epubs unchanged. Is that the case?

This leads me to be optimistic:

It's still sitting like that now awaiting a kind forum user to help me out.

Thank you!

Correct, just click No, and relax

DoctorOhh · 07-03-2012, 07:40 AM

Quote:

Originally Posted by 44reader

I've accidentally run "smarten punctuation" on all my files.

I'm at the box where it says "Modify ePub modified x file(s). Proceed with updating your library?'

I'm really hoping that clicking No will leave my epubs unchanged. Is that the case?

If you don't want "smarten punctuation" run then, other than pulling the plug on your machine, click no. It will ether stop the process or not, but clicking yes most certainly will proceed to update your library.

44reader · 07-03-2012, 08:05 AM

Quote:

Originally Posted by jackie_w

Correct, just click No, and relax

Nice!

I haven't got many books so it wouldn't have killed me but that is a relief.

I do wonder about the wording of the message though. "Modify ePub modified 123 files" sounds like it has already modified the files.

I know Kiwidude will have good reason for wording it like this and I look foward to hearing that.

Thank you Kiwidude very very much for all your great plugins. You really are da man, Dude.

Thanks, jackie_w for the quick help.

[And thanks, dwanthny, for the quick answer too - even though I did realize that...

Quote:

Originally Posted by dwanthny

It will ether stop the process or not, but clicking yes most certainly will proceed to update your library.

]

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
[GUI Plugin] Quality Check	kiwidude	Plugins	1184	04-17-2024 06:17 PM
[GUI Plugin] Open With	kiwidude	Plugins	403	04-01-2024 08:39 AM
[GUI Plugin] Manage Series	kiwidude	Plugins	166	02-13-2024 11:31 AM
Modify ePub plugin dev thread	kiwidude	Development	346	09-02-2013 05:14 PM
[GUI Plugin] Plugin Updater Deprecated	kiwidude	Plugins	159	06-19-2011 12:27 PM

07-02-2012, 01:02 PM	#188
ldolse Wizard Posts: 1,337 Karma: 123455 Join Date: Apr 2009 Location: Malaysia Device: PRS-650, iPhone	@JSWolf, As I said before, the plugin code inserts @page by design - this is the only way to insert top and bottom page margins to match Calibre's preferences - this is the only way you can use modify ePub to fix the corresponding check in quality check. Inserting @page in general is not a bug and it's not going to change. Aside from offending your sensibilities, an empty @page declaration is going to behave exactly the same as not inserting @page, but I'm ok with calling this case an issue, albeit a low priority one since it won't affect how the book renders. Whenever I get a chance to dig back into the plugin to investigate Jackie's case I'll look into not inserting empty @page statements when all margins are zero.

07-02-2012, 09:12 PM	#191
JSWolf Resident Curmudgeon Posts: 73,897 Karma: 128597114 Join Date: Nov 2006 Location: Roslindale, Massachusetts Device: Kobo Libra 2, Kobo Aura H2O, PRS-650, PRS-T1, nook STR, PW3	Kiwidude, can we have an option to delete @page please?