Skip to content

CSS: add support for "ruby-position: over/under/alternate"#652

Merged
poire-z merged 2 commits into
koreader:masterfrom
poire-z:ruby_position
Feb 14, 2026
Merged

CSS: add support for "ruby-position: over/under/alternate"#652
poire-z merged 2 commits into
koreader:masterfrom
poire-z:ruby_position

Conversation

@poire-z

@poire-z poire-z commented Feb 14, 2026

Copy link
Copy Markdown
Contributor

No support for "inter-character".
https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/ruby-position
https://drafts.csswg.org/css-ruby-1/#rubypos
Should allow closing koreader/koreader#14970.

(Also add forgotten inheritance flag to caption-side.)

Done with the help of Github copilot by guiding it step by step, full Copilot conversation/copiloting in poire-z#3 (18 conversation posts, 9 commits).

Also include: MathML: don't handle anything when no def style set
Fix crash when called from getBalancedHTML() (where no style is set) and meeting MathML elements. We don't need to do any of our MathML handling (adding attributes, wrappping in MathBox) when in this context, known by the fact the document has no style set.
Should allow closing koreader/koreader#14974


This change is Reviewable

No support for "inter-character".
(Also add forgotten inheritance flag to caption-side.)
@poire-z

poire-z commented Feb 14, 2026

Copy link
Copy Markdown
Contributor Author

Keeping here the full description Copilot has updated in the top post of poire-z#3, for reference:

This PR adds classic CSS parsing/handling for ruby-position and implements ruby annotation row reordering in rendering.

What’s included

  • Added CSS ruby-position support for:
    • alternate
    • over
    • under
  • Added style-model plumbing for ruby_position:
    • enum definition
    • style record field
    • important-bit handling
    • hash/equality integration
    • serialization/deserialization integration
  • Wired stylesheet parsing and application for ruby-position.
  • Set correct initial/default behavior:
    • parser initial maps to alternate
    • root style default includes ruby_position = css_rp_alternate
  • Added missing inheritance application flag alignment for related style handling.
  • Bumped formatting version to ensure reformatting with the new style field.
  • Added ruby_position propagation in rendering style copy/inheritance paths.

Rendering behavior implemented

Ruby table rows are reordered visually based on each annotation row’s own style->ruby_position (row-like children such as RBC/RTC/RubyBox), with the base row kept as the logical first child.

Final logic is a simplified single-pass approach:

  • Iterate annotation rows from index 1.
  • Track previous resolved side with boolean state (prev_was_over).
  • Resolve alternate as opposite of previous resolved side.
  • Move resolved over rows to position 0 during iteration.
  • Leave resolved under rows in flow order.

This preserves expected stacking behavior around the base while keeping the implementation compact.

Notes

  • inter-character is intentionally not implemented.
  • Changes are focused on classic CSS/style/rendering paths needed for ruby positioning.

@Frenzie Frenzie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cleaner than I'd expect from an LLM (but that's probably you ;-)

@poire-z

poire-z commented Feb 14, 2026

Copy link
Copy Markdown
Contributor Author

It was quite simple, mostly some copy & paste of some other similar property (the AI actually picked the most similar one, caption-side), but it still forgot a few things.
And for the actual rendering, it did find out on its own where it should happen. It went complex (using intermediate arrays to order the rows), while it was just a matter of iterating and moving any "over" at position 0.

I tried ChatGPT "Codex" this time, and it was a lot less cherful than Claude :)
Instead of enthousiasming "You are absolutely right!", I got just "Yes, and I did that in commit xyz" :/

Fix crash when called from getBalancedHTML() (where no style is set)
and meeting MathML elements. We don't need to do any of our MathML
handling (adding attributes, wrappping in MathBox) when in this
context, known by the fact the document has no style set.
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