Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unusual placement of list elements #521

Closed
gjtoth opened this issue Jun 12, 2023 · 9 comments · Fixed by #555 or koreader/koreader#11527
Closed

Unusual placement of list elements #521

gjtoth opened this issue Jun 12, 2023 · 9 comments · Fixed by #555 or koreader/koreader#11527

Comments

@gjtoth
Copy link

gjtoth commented Jun 12, 2023

KOReader seems to place the list elements in a strange way. Most browsers and book readers I'm aware of make sure that the outside markers are visible by either setting margin or padding of either the ul/ol or li elements. If all these are set to zero, the markers are off the screen. It is not, however, so in KOReader. Could this be made consistent with other players?

Attached are a simple epub file and screenshots made in KOReader and Calibre Book Viewer.

Any support would be appreciated.

--Gabor

NB: the zip file is the epub file, just renamed, to allow uploading.

test.epub
Screenshot of Calibre Book Viewer
Screenshot of KORader

@poire-z
Copy link
Contributor

poire-z commented Jun 12, 2023

crengine does this from before I started hacking it, and when making "enhanced block rendering", I didn't feel like changing it.
It is indeed not per specs, so what you're trying to achive (hiding bullets/numbers out in the margin?) does not work.
Per-specs, I guess other renderers have to set some margins (and set some default) to push the bullets/numbers in, and it often/always feel misaligned, and can be messy when the list item numbers are larger than the margin. And it is somehow hard to fight
(cf. https://github.com/koreader/koreader/blob/f7d5828f084827f3011078a9c3ebb1cbf27cf148/frontend/ui/widget/dictquicklookup.lua#L782-L785) when you want to have it all in the view.

The way crengine has always been doing it is to compute the max width of all bullet/numbers, and use that as some kind of "INNER margin" for the list item content. It's not per-specs, but it mostly always do the right and nice things (but ok, preventing you and publishers from achieving what they want in some cases.) I don't think I want to change that.

And if you want to fight that with style tweaks, you're also a bit doomed for the same reason that you don't know the amount of computed needed margin, so you can't set a proper negative margin to counteract ours :/
So, I guess that if you want to hide the bullet/numbers with style tweaks, the answer is display: block :)

@gjtoth
Copy link
Author

gjtoth commented Jun 12, 2023

Thanks for the quick response!

As for my goal, it's nothing extraordinary: I'd like control over the indentation of my lists and in a way that is consistent across readers.

I agree that display: block can achieve that. The reason I'm reluctant to go that way is ordered lists – for those, I'd either need to burn the markers in or rely on css counters. And I'm not sure how consistently the latter is supported.

Would maybe adding a vendor-specific parameter (such as -cre-use-inner-margin) be feasible?

@poire-z
Copy link
Contributor

poire-z commented Jun 12, 2023

Feasible to trigger our default not-per-specs behaviour with a vendor property: yes - but implementing the per-specs behaviour: not fun :) (but it could be interesting, but anyway, not anytime soon :/)
And anyway, you'll get our default behaviour per default, so you'd have to find a trick to disable it (or us to provide something to allow that).
(I realize I haven't even thought about adding a vendor @media property so one could use @media koreader or crengine { CSS only for KOReader)).

Or may be you could use UL { list-style-type: none } ?

@gjtoth
Copy link
Author

gjtoth commented Jun 12, 2023

A media query would be quite OK for what I need. Could that be implemented in the foreseeable future?

@poire-z
Copy link
Contributor

poire-z commented Jun 12, 2023

A media query would be quite OK for what I need. Could that be implemented in the foreseeable future?

Just a media query to find out crengine ? And no need for a per-specs list item margin implementation ?
If that, yes, that's easily doable.
What would you put under it (so, only CSS we support) to get the behaviour you want ?

Btw, you may already be able to detect KOReader vs others. For some rare @media properties, we report we are dumb :) Others renderer/apps may show off and report they are clever :)

"monochrome", // For now, we'll say we're not monochrome, even on eInk
"grid", // false, we're not a tty terminal with a fixed font
"scripting", // "none", we don't support scripting
"update", // "none", "Once it has been rendered, the layout can no longer be updated"
"overflow-inline", // "none", no horizontal scrolling
"overflow-block", // "paged", which is our main purpose (even if we have a scroll mode)

Or maybe even @supports !
image
(this CSS, even if the name feels like it would do what you want, won't help you: #462 - it's just a random pick of one of our vendor specific property).

@gjtoth
Copy link
Author

gjtoth commented Jun 12, 2023

Gut feeling is that making li's display: block, with suitable left-margin, the same but negative text-indent, CSS counters for ordered lists, and a bit of ::before magic, might do the trick. Need to verify, though.

@poire-z
Copy link
Contributor

poire-z commented Jun 12, 2023

fyi: crengine does not support CSS counters.

@poire-z
Copy link
Contributor

poire-z commented Feb 19, 2024

Giving this some thoughts, as it's one of the last little bits we are obviously not per-specs.

I think we can become per-specs with just commenting out a few things in the code, ie.:

--- a/crengine/src/lvrend.cpp
+++ b/crengine/src/lvrend.cpp
@@ -8204,3 +8204,3 @@ void renderBlockElementEnhanced( FlowState * flow, ldomNode * enode, int x, int
                         // below when calling renderBlockElement() for each child
-                        list_marker_padding = list_marker_width;
+//                        list_marker_padding = list_marker_width;
                     }
@@ -8524,2 +8524,3 @@ void renderBlockElementEnhanced( FlowState * flow, ldomNode * enode, int x, int
                 int list_marker_padding = 0;;
+if (true) { } else
                 if ( style->display == css_d_list_item_block ) {
@@ -10094,2 +10095,3 @@ void DrawDocument( LVDrawBuf & drawbuf, ldomNode * enode, int x0, int y0, int dx
                     if ( is_rtl ) {
+shift_x += list_marker_width;
                         txform->Draw( &drawbuf, doc_x+x0 + width + shift_x - list_marker_width, doc_y+y0 + padding_top );
@@ -10098,2 +10100,3 @@ void DrawDocument( LVDrawBuf & drawbuf, ldomNode * enode, int x0, int y0, int dx
                         // (Don't shift by padding left, the list marker is outside padding left)
+shift_x -= list_marker_width;
                         txform->Draw( &drawbuf, doc_x+x0 + shift_x, doc_y+y0 + padding_top );

KOReader current | Firefox | KOReader with this patch:
image
(ignore the background coloring of list numbers, this should probably go away.)

That's with html5.css that has:

ol, ul {
  padding-left: 40px;
}

The HTML specs and current web browsers still have this in their usage agent stylesheet.

It feels a bit absurd to use this hardcoded absolute value of 40px.
Per-specs, the bullet or list item numbering should be put outside of the <li> border box, so either the list item has to provide some margin-left, or the parent elements some padding-left or margin-left, otherwise these bullets or numbers could end up in the page margins or even outside and get truncated (or if list items used in tables, outside their table cell and present in an adjacent cell...).
So, I guess that's the reason for providing a default padding-left for ol, ul, and 40px to at least get something. But depending on the font size used for that bullet or numbers, it can still go out in the margin if a large font is used and 2 digits take more than 40px - or provide too much indendation if a small font size is used.
I guess the advantage of this default of 40px is that all list items content gets vertically aligned, no matter the font-size or bullet/number size, so overall it may look neat (but not caring about bullets/numbers getting truncated).

Wikipedia on their web pages override this default with padding-left: 1.6em which might be enough to display 2 or 3 digits. I think all publishers that care about rendering have to override this default of 40px (or use other tricks, like just putting some margin-left on <li> and not care about ol, ul.
Also, anything can be display: list-item, so targetting/fixing ol, ul, li is just one use-case (taken care of or corrupted :))

I don't want to throw out the crengine sweet way of doing it (computing a dynamic padding based on the widest bullet/number used in the list), as it can sometimes do the best thing (dunno how authors can achieve that in web pages, either by using tables or javascript to compute that required width), and we need it for Wikipedia footnotes, as they are <ol><li>.

We could provide the current crengine behaviour with another -cr-hint: (so, not by default, can be available to fix stuff, which would also need the tweak to kill any padding/margin), or may be automagically by using in our html5.css and epub.css:

ol, ul {
  padding-left: auto;
  /* or a private -cr-list-auto , but 'auto' is not accepted with padding, so it is
      fine to use, and I get its parsing for free :) */
}

As, as I wrote above publishers that care about rendering have to override this default (either to provide a more logical value like 1.6em, or to put it at 0 if they wish to ensure that spacing some other way), they would end up overriding my padding-left: auto with their tuned value. And if they don't (like in my Wikipedia EPUBs, where I currently force that to 0, but I would remove that, or I would use this auto - or in any unstyled EPUB just using OL/UL/LI), we would still get the magical dynamic autosizing of that padding.

Thoughts ?

@Frenzie
Copy link
Member

Frenzie commented Feb 19, 2024

Shouldn't that be something like padding-left: -cr-auto, or would that mess with your for free. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants