View Single Post
Old 07-02-2012, 05:03 AM   #183
ldolse
Wizard
ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.ldolse is an accomplished Snipe hunter.
 
Posts: 1,337
Karma: 123455
Join Date: Apr 2009
Location: Malaysia
Device: PRS-650, iPhone
Quote:
Originally Posted by jackie_w View Post
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 View Post
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 View Post
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).

Last edited by ldolse; 07-02-2012 at 05:16 AM.
ldolse is offline   Reply With Quote