Skip to content

Fix RoyalRoad parser for the site's Tailwind redesign#2786

Open
possibletoactual wants to merge 1 commit into
dteviot:ExperimentalTabModefrom
possibletoactual:royalroad-redesign-fix
Open

Fix RoyalRoad parser for the site's Tailwind redesign#2786
possibletoactual wants to merge 1 commit into
dteviot:ExperimentalTabModefrom
possibletoactual:royalroad-redesign-fix

Conversation

@possibletoactual

Copy link
Copy Markdown

Royal Road's site redesign broke RoyalRoadParser: on the new markup the chapter list silently collapses to a single chapter, and author/cover/description/genres extraction fails. This restores the parser against the new markup while keeping the old selectors as fallbacks.

What changed

  • Chapter list — parse the window.chapters JSON array (present in every page variant) instead of the removed table#chapters; fall back to the legacy table.
  • Chapter content — anchor on div.chapter-inner (the portlet-body wrapper is gone) and null-guard so a single odd chapter can't throw and abort the whole download.
  • Chapter titles — use the authoritative title from the chapter list instead of the first <h1> (now an unrelated "Give Reputation to User" widget).
  • Author — header /profile/ link → books:author meta → legacy selector.
  • Coverog:image / twitter:image → legacy img.thumbnail.
  • Description / genres / information page — the served DOM has scripts stripped (no JSON-LD) and og:description is truncated, so the full description is scraped from the raw page during analyse; genres read from a[href*='tagsAdd=']. The information page is rebuilt with title, author, genres, and full description.

PR checklist

  • Unit tests: pass — existing UtestRoyalRoadParser covers removeOlderChapterNavJunk / makeHiddenElementsVisible, which are unchanged.
  • eslint: clean.
  • New behavior: none — this restores existing behavior broken by the site change, so there's no new user-facing toggle.
  • Contributors / Credits: added to package.json and readme.md.
  • Base branch: ExperimentalTabMode.

Legacy selectors are preserved as fallbacks, so older/cached page variants continue to work.

Royal Road's redesign broke the RoyalRoadParser: chapters silently dropped to
one, and metadata/content extraction failed. This repairs it against the new
markup while keeping the legacy selectors as fallbacks:

- Chapter list: parse the `window.chapters` JSON array (present in every
  variant) instead of the removed `table#chapters`; fall back to the table.
- Chapter content: anchor on `div.chapter-inner` (the `portlet-body` wrapper is
  gone) and null-guard so one odd chapter can't abort the whole book.
- Chapter titles: use the authoritative title from the chapter list rather than
  the first `<h1>` (now an unrelated "Give Reputation to User" widget).
- Author: header `/profile/` link, then `books:author` meta, then legacy.
- Cover: `og:image` / `twitter:image`, then legacy `img.thumbnail`.
- Description/genres/information page: scrape the full description from the raw
  page (og:description is truncated; JSON-LD is stripped from the served DOM)
  and read genres from `a[href*='tagsAdd=']`; rebuild the info page with title,
  author, genres, and full description.

Unit tests unaffected (they cover other methods); eslint clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
// Capture book metadata (description + genres) from the RAW html now — the DOM
// the parser later sees has its <script> tags stripped and, on the redesign, no
// JSON-LD at all, so the full description must be scraped here.
RoyalRoadParser.lastBookMeta = RoyalRoadParser.extractBookMeta(tocHtml);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't store book meta in a static class prop, I'm pretty sure this will cause a bug if you queue 2 RR books at once.
Instance prop this.lastBookMeta or could attach it to the DOM i.e. dom.__royalRoadMeta

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@kuwoyuki

I'm not sure how you'd queue 2 RR books at once? If you open two copies of WebToEpub, they're two instances. With their own statics.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, yeah you're right, each popup is a separate document.
I still think this is an anti-pattern though, because every method that writes or reads lastBookMeta already has this. So it's the wrong lifetime.. metadata is per-parse, per-book. A static is per realm (popup).

// Extract the chapter list from the page's `window.chapters = [...]` JSON blob.
// Returns [] if not present/parseable, so the caller can fall back.
static chaptersFromWindowChapters(html, baseUrl) {
let match = html.match(/window\.chapters\s*=\s*(\[[\s\S]*?\])\s*;/);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[\s\S]*? is lazy.
what will happen if chapter is named Chapter 1 [Interlude]?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@possibletoactual

You should probably have a look at util.locateAndExtractJson()

Comment thread plugin/js/parsers/RoyalRoadParser.js
// Prefer JSON-LD when present; otherwise locate the full description in the body by
// matching the og:description as a needle (the redesign's classes are unstable).
static extractBookMeta(html) {
let doc = new DOMParser().parseFromString(html, "text/html");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

since you parsed it here anyway, you can pass it to getChapterUrls for the fallback extraction, to avoid parasing the DOM twice

}
let best = null;
let bestLen = 0;
for (let el of doc.querySelectorAll("div, section, article")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you narrow down the search? Iterating over every div, section, article is kinda meh, if there's some heuristic that can be used it would be nice

// formatting (sanitized downstream by populateInfoDiv).
nodes.push(heading);
let descDiv = dom.createElement("div");
descDiv.innerHTML = descriptionHtml;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

No NOT assign un-sanitized HTML to innerHTML. It makes approval for Mozilla and Google stores difficult.
Better yet, do not assign to innerHTML at all.

?.getAttribute("content") ?? "";
let el = RoyalRoadParser.findDescriptionElement(doc, og);
if (el != null) {
descriptionHtml = el.innerHTML;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Do not assign to innerHTML.

if ((jsonld != null) && !util.isNullOrEmpty(jsonld.description)) {
descriptionHtml = jsonld.description;
let holder = doc.createElement("div");
holder.innerHTML = jsonld.description;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

do not assign to InnerHTML.

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.

3 participants