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

Force BR to always be display:inline #338

Merged
merged 1 commit into from
Apr 19, 2020

Conversation

poire-z
Copy link
Contributor

@poire-z poire-z commented Apr 19, 2020

BR shouldn't be handled just like any block element (a height applied to it is not ensured by Firefox or Calibre).
We get more similar results with Firefox/Calibre by just forcing all BR to be display: inline.
Revert a bit of 36ae6cc (from #337) (but let the logic in for <empty-line>).

More details in koreader/koreader#6069 (comment) and #172 (comment).


Thought about removing the height in epub.css:

/* Element added for each empty line when rendering plain txt files */
empty-line {
    height: 1em;
}

to keep lines aligned with .txt documents - but they use PRE for each line, which has some top and bottom margins, so preventing any constant line-height alignment.

But just noticed that if you "Clear all external styles" with txt document, you get an alternate rendering, without monospace fonts, no margin for PRE, no height for empty-line, so something that I find more readable and that has constant line-height aligmnent:

image

I'd like to rename that menu item: Clear all external styles to just None.
That long sentence is a bit unclear about what it does, what are external styles, why the plural (ok, it's many styles in a single user agent CSS), and styletweaks still work when this is checked.
Thoughts?


This change is Reviewable

BR shouldn't be handled just like any block element (a height
applied to it is not ensured by Firefox or Calibre).
We get more similar results with Firefox/Calibre by just
forcing all BR to be display: inline.
Revert a bit of 36ae6cc (but let the logic in for <empty-line>).
@Frenzie
Copy link
Member

Frenzie commented Apr 19, 2020

Yes, I like "None".

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

Successfully merging this pull request may close these issues.

2 participants