FB2: Make single image a block level element

[dmalinovsky]

When we have a paragraph which contains only a single image inside, it makes sense to make this image a block element. Currently, it's marked as inline, which is good for inline image formulas, etc., but not for the use case below.

The paragraph contains a single image, nothing else.

Before this change	After

As you can see, previously the image was cropped at the right border and indented.

---

This change is 

[poire-z]

You FB2 readers will have to look at other books, to see if any uses <image> inside <p> expecting them to be inlined.
Because for me from the HTML world, a <p> is a block element that is a direct container of text and other inline elements. So it feels right that an <image> inside a <p> gets the CSS bit (that you're removing), so it overrides the one before that sets it display:block by default.

If you feel like you need it on that book, it may be the book that was badly made ? It feels it could make it worse on other books.

[dmalinovsky]

Hmm, it is possible to put image outside of p, that's true. Perhaps I need to change my custom CSS instead.
Let's see what the others will say.

[dmalinovsky]

Also, the reason for this change is that currently the image is truncated at the right margin, which shouldn't happen.

[Frenzie]

Also pinging @mergen3107
koreader/koreader#11623

[mergen3107]

I was happy with my solution there for centering books. With this PR, will my solution break?

[Frenzie]

Like @poire-z I'm not convinced that the change in this PR is desirable. I was mainly just hoping you'd have some insight about the subject in general.

[mergen3107]

I was mainly just hoping you'd have some insight about the subject in general.

Sorry, I am more a mere tinkerer :D
If a thing doesn't look right I tinker long enough with CSS until it works.

I have no idea whether it breaks some rules or how it normally should be :D

Too many books are created wrong to expect them to behave, unfortunately.

[dmalinovsky]

After some thinking, I believe my solution isn't correct. It'll break image formulas in the paragraphs, for example — instead of displaying them inline, it'll put them on separate lines.

It's better to add this line:

p image:only-child { display: block; }

This way paragraphs with the only image will display it correctly.

@poire-z, will it work fast enough?

[Frenzie]

That works for the OP? I thought that was two images in one p.

[dmalinovsky]

That works for the OP? I thought that was two images in one p.

Yes, it works, it’s a single image without anything else.

[Frenzie]

If it's a single image pretending to be two then that seems sensible enough.

[dmalinovsky]

The problem is not it's pretending, but it has horizontal orientation and its right edge is clipped, because the image is moved to the right.

[Frenzie]

The problem is not it's pretending

But it is because I doubt I'm the only one who thought there were two images. ;-)

[dmalinovsky]

The problem is not it's pretending

But it is because I doubt I'm the only one who thought there were two images. ;-)

I'm sorry, I didn't realize that, but you're right, of course. :) It's a single image combining two illustrations.

[dmalinovsky]

I've updated the PR and its description.

[poire-z]

It's better to add this line:

p image:only-child { display: block; }

Not better, just not good :/

I don't think you can detect what you want with CSS (at least with normal CSS - may be we can with one of our private extension to CSS, see bottom). Any workaround for this "issue" would have to be in the code. If it is really an issue.

it has horizontal orientation and its right edge is clipped, because the image is moved to the right.

That shift is caused by the text-indent. The image is shifted, its sizing is not changed because of this. If it had a few lines before it in the paragraph, you would not want its size changed: you would break a line, and have the image take the full width - you wouldn't want it to be smaller.

This happens also with HTML and in web browsers (tested with Firefox: when there is text-indent, if the image is set width:100%, it is indented, shifted, and overflows on the right: you get some horizontal scroll bar, and if you scroll to the right, you can see the image takes the viewport width (but because of the text-indent, the content width is larger than the viewport width, so the scrollbar).
That's the per-specs behaviour of an inline image in a P, and it sucks indeed. In these situations, I think one can help himself with a style tweak setting img { max-width: 95% } and hope it's enough to counteract the text-indent.

In crengine, we additionally ensure an image can't take more of the container (or screen, don't remember) width, so we cap it to that. But we don't account for the text-indent, because we don't know if it's on the first line or another line (see paragraph above).

So, for HTML, we are per specs.
FB2 is crappy in that there is no real specs. It re-uses HTML, but the structure it allows is non-conforming and the expected rendering not specified (ie <title> here and there without telling how it should render).
I don't know if or what it says about IMG inside P.
That's why I asked to check if standalone IMG in a P is something common in other FB2 books..
If other books put IMG in-between Ps or in sections or other tags, it's not something common, and it's a publisher error, and it doesn't need any fix by us.

May be a user can fix it with a style tweak. This seems to work when used in a style tweak:
p:not([_]) img { display: block; border: 1px solid red}
(p:not([_]) , something private we added recently) allows targetting a P without any text.)
In our code:

    if ( useragent_sheet && attrname[0] == '_' && attrname[1] == 0 ) {
        // crengine private syntax (only allowed in useragent stylesheet (which
        // includes styletweaks): '_' as the attribute name means we want to
        // match against the node's full inner text.
        id = attr_InnerText;
        // When applying styles while in the initial book loading and DOM building phase,
        // no text is available yet, so this does need a re-rendering.
        // (All the initial checks will fail, quickly and cheaply, no need to try to avoid them.)
        doc->setNodeStylesInvalidIfLoading(NODE_STYLES_INVALID_PECULIAR_CSS_INNER_CONTENT_CHECK);
    }

so this trick may work if set in fb2.css (but not if set in the book, wasted a few minutes trying it in the <head><style> of my test html file).
But the side effect will be that all FB2 will need a re-rendering... so, it will take ~ 1.5 more time to load - for maybe fixing a publisher error.

[dmalinovsky]

I see, thanks for the detailed explanation, @poire-z! I don't know how common is such code, but KOReader is doing a right thing here. I'll close this PR then and try to fix this issue with the user style tweak.