diff --git a/custom-css-analysis-and-migration/README.md b/custom-css-analysis-and-migration/README.md index e66fab2..b13e872 100644 --- a/custom-css-analysis-and-migration/README.md +++ b/custom-css-analysis-and-migration/README.md @@ -1,6 +1,6 @@ In Bloom 5.7, we introduce a modernized, parameterized page layout CSS system, known as "Appearance". This new system conflicts with the arbitrary custom css that was used in prior Bloom versions, mostly in the area of margins. -As of this writing, we are only looking at `customBookStyles.css`, but `customCollectionsStyles.css` have the same problem. +As of this writing, we are looking at both `customBookStyles.css` and `customCollectionsStyles.css`, but not handling having both files with content, and not really dealing with `customCollectionStyles.css` being a collection wide settings file. When Bloom encounters a pre-5.7 book with a `customBookStyles.css` that would conflict with the new system, it goes into a kind of "safe mode" that keeps things working by using a legacy `basePage.css`. Better, though, is to migrate the old css to the new system. Conceivably, a reliable program could be written to do this automatically. However at the moment what we do is to write the migrations using the utilities here, and then have a dev evaluate individual migrations and then copy them over to the shipping Bloom. So when Bloom encounters one of these books, we may already have a tested migration for it. @@ -8,26 +8,28 @@ When Bloom encounters a pre-5.7 book with a `customBookStyles.css` that would co ⚠️ Be careful what you commit to an open source repo. We do not want to expose emails or other private data. -1. As of this writing, bun only works on linux. If you are on windows, just install Windows Subsystem for Linux (not as big of a deal as it sounds), then run in a WSL terminal in VSCODE. +1. As of this writing, bun works only on linux. If you are on windows, just install Windows Subsystem for Linux (not as big of a deal as it sounds), then run in a WSL terminal in VSCODE. -1. Get [bun](https://bun.sh/) installed +2. Get [bun](https://bun.sh/) installed -1. Install dependencies: `bun install` - -1. Download all the `customBookStyles.css` from blorg +3. Choose (or create) a folder for working with CSS files and download all the `customBookStyles.css` and `customCollectionStyles.css` files from bloomlibrary.org. This will take some time. `./some-path/BloomBulkDownloader.exe --include "*/*/custom*Styles.css" --syncfolder ./output/downloads --bucket production customBookStyles-files` +4. Download all the `meta.json` files from bloomlibrary.org. Again, this will take some time. + + `./some-path/BloomBulkDownloader.exe --include "*/*/meta.json" --syncfolder ./output/downloads --bucket production customBookStyles-files` + Each of the following take an optional argument that will limit the number of records processed. -5. Process those, grouping them into bins of duplicate stylesheets +5. Process those, grouping them into bins of duplicate stylesheets - `bun run group-stylesheets.ts 13` + `bun run /path-to/group-stylesheets.ts 13` -1. Process those groups, discarding ones that don't need migration +6. Process those groups, discarding ones that don't need migration - `bun run filter-stylesheets.ts 7` + `bun run /path-to/filter-stylesheets.ts 7` -1. Create draft migration files for each one +7. Create draft migration files for each one - `bun run create-migrations.ts 3` + `bun run /path-to/create-migrations.ts 3` diff --git a/custom-css-analysis-and-migration/create-migrations.ts b/custom-css-analysis-and-migration/create-migrations.ts index 01200e7..8ec99f7 100644 --- a/custom-css-analysis-and-migration/create-migrations.ts +++ b/custom-css-analysis-and-migration/create-migrations.ts @@ -36,25 +36,47 @@ for (const record of records) { if (record.css === undefined) continue; ++count; - if (Bun.argv.length > 2 && count >= Number.parseInt(Bun.argv[2])) break; - console.log(`css = ${record.css}`); + if (Bun.argv.length > 2 && count > Number.parseInt(Bun.argv[2])) break; + //console.log(`css = ${record.css}`); const checksum = crypto.createHash("md5").update(record.css).digest("hex"); const folder = `./output/migration/${count} ${checksum}`; + console.log(`output folder=${folder}`); fs.mkdirSync(folder); + let cssPath = `${folder}/customBookStyles.css`; + + const migration = migrateCssToAppearance(record.css); + fs.writeFileSync( - `${folder}/customBookStyles.css`, - migrateCssToAppearance(record.css) + - `\n\n /* ----- ORIGINAL ---- */\n/*${record.css.replaceAll( - "*/", - "#/" - )}*/` + cssPath, + migration.modifiedCss + + ` + +/* ----- ORIGINAL ---- */ +/*${record.css.replaceAll("*/", "#/")}*/ +` ); + const brandingSet = new Set(); + for (let i = 0; i < record.paths.length; ++i) { + const metaPath = "./output/downloads/" + record.paths[i] + "/meta.json"; + try { + const metaString: string = fs.readFileSync(metaPath, "utf8"); + const metadata = JSON.parse(metaString); + const brandingProject = metadata.brandingProjectName as string; + if (brandingProject && + brandingProject.toLowerCase() !== "default" && + brandingProject.toLowerCase() !== "local-community") + brandingSet.add(brandingProject); + } catch (e) { + console.log("Could not extract brandingProjectName from " + metaPath + ": " + e); + } + } + const uniqueUploaders = [ ...new Set( record.paths.map((path: string) => { - const email = path.split("/")[2]; + const email = path.split("/")[0]; const emailParts = email.split("@"); const obfuscatedEmail = emailParts[0].slice(0, -2) + "..." + "@" + emailParts[1]; @@ -63,15 +85,25 @@ for (const record of records) { ), ].slice(0, 3); // first 3 are enough to give a sense of who uploaded these books - fs.writeFileSync( - `${folder}/appearance.json`, + const brandings = [...brandingSet]; + const date = new Date(); + const outputString: string = `// Matches customBookStyles.css with checksum ${checksum} -// This was used by ${record.book_count} books (${record.unique_named_books} unique).` + - // enhance: +`// Affected branding projects include "Mali-ACR-2020-Soninke".` - `// Uploaders included ${JSON.stringify(uniqueUploaders)}. +// On ${date.toDateString()} this was used by ${record.book_count} books (${record.unique_named_books} unique). +` + + (brandings && brandings.length > 0 + ? `// Affected branding projects included ${JSON.stringify(brandings)}. +` + : ``) + + `// Uploaders included ${JSON.stringify(uniqueUploaders)}. // Example Book: ${record.first_book} // A replacement customBookStyles.css has been generated. { -}` - ); + "cssThemeName": "default", +} +`; + let appearancePath = `${folder}/appearance.json`; + if (fs.existsSync(appearancePath)) + appearancePath = `${folder}/appearance2.json`; + fs.writeFileSync(appearancePath, outputString); } diff --git a/custom-css-analysis-and-migration/filter-stylesheets.ts b/custom-css-analysis-and-migration/filter-stylesheets.ts index 71c7841..8ce453a 100644 --- a/custom-css-analysis-and-migration/filter-stylesheets.ts +++ b/custom-css-analysis-and-migration/filter-stylesheets.ts @@ -15,7 +15,7 @@ const count = records.reduce((acc, record) => acc + record.paths.length, 0); console.write(`total books with custom css rules: ${count}\r\n`); console.write(`total unique css files: ${records.length}\r\n`); -const kProbablyWillInterfere = `\\.marginBox\\s*\\{[^\\}]*?(? 2 ? Number.parseInt(Bun.argv[2]) : 10000000; @@ -34,7 +34,7 @@ const recordsWithUniqueifiedPaths = filteredRecords.map((record) => { const uniquePaths = uniqueFilenames.map((filename) => { return paths.find((path) => path.endsWith(filename)); }); - const instanceId = paths[0].split("/")[2]; + const instanceId = paths[0].split("/")[1]; return { book_count: paths.length, unique_named_books: uniquePaths.length, @@ -75,12 +75,13 @@ console.write( `, covering ${filteredRecords.reduce( (acc, record) => acc + record.paths.length, 0 - )} books, ` + )} books` ); - // console.write( -// `${recordsWithUniqueifiedPaths.reduce( +// `, ${recordsWithUniqueifiedPaths.reduce( // (acc, record) => acc + record.uniqueified_paths.length, // 0 // )} of which have unique names (should remove most rebrands).\r\n` // ); +console.write(` +`); diff --git a/custom-css-analysis-and-migration/group-stylesheets.ts b/custom-css-analysis-and-migration/group-stylesheets.ts index 8aa80ea..4958674 100644 --- a/custom-css-analysis-and-migration/group-stylesheets.ts +++ b/custom-css-analysis-and-migration/group-stylesheets.ts @@ -11,35 +11,38 @@ interface FileData { } const boilerplate = - '/* Some books may need control over aspects of layout that cannot yet be adjusted\r\n from the Bloom interface. In those cases, Bloom provides this "under the hood" method\r\n of creating style rules using the underlying "Cascading Stylesheets" system.\r\n These rules are then applied to all books in this collection. EDIT THIS FILE ONLY\r\n IN THE COLLECTION FOLDER: changes made to a copy found in the book folder will be\r\n lost the next time the book is edited with Bloom!\r\n\r\n Note: you can also add a file named "customBookStyles.css" in the book folder,\r\n to limit the effects of the rules to just that one book.\r\n\r\n You can learn about CSS from hundreds of books, or online. However chances are, if\r\n you need this customization, you will need an expert to create a version of this file\r\n for you, or give you rules that you can paste in below this line. */'; + /\/\* Some books may need control over aspects of layout that cannot yet be adjusted(.|[\r\n])*?\*\//; -const boilerplate2 = - '/* Some books may need control over aspects of layout that cannot yet be adjusted\r\n from the Bloom interface. In those cases, Bloom provides this "under the hood" method\r\n of creating style rules using the underlying "Cascading Stylesheets" system. \r\n These rules are then applied to all books in this collection.\r\n\r\n Note: you can also add a file named "customBookStyles.css" in the book folder,\r\n to limit the effects of the rules to just that one book.\r\n \r\n You can learn about CSS from hundreds of books, or online. However chances are, if\r\n you need this customization, you will need an expert to create a version of this file\r\n for you, or give you rules that you can paste in below this line. */'; let count = 0; function readFilesRecursively( dir: string, fileMap: Map ): void { const files = fs.readdirSync(dir); + let cssCount = 0; for (const file of files) { const filePath = path.join(dir, file); if (Bun.argv.length > 2 && count >= Number.parseInt(Bun.argv[2])) return; if (fs.statSync(filePath).isDirectory()) { readFilesRecursively(filePath, fileMap); } else { + if (file === "meta.json") continue; const content = fs .readFileSync(filePath, "utf8") .replace(boilerplate, "") - .replace(boilerplate2, "") .trim(); if (content === "") continue; + ++cssCount; const fileData: FileData = fileMap.get(content) || { content, paths: [] }; - fileData.paths.push(dir.replace("./output/downloads", "")); + fileData.paths.push(dir.replace(/^(\.\/)?output\/downloads\//, "")); fileMap.set(content, fileData); - console.log(++count + " " + dir); + console.log(++count + " " + filePath); } } + if (cssCount > 1) + console.log(`INFO: customCollectionStyles.css and customBookStyles.css both have content in ${dir}`); + cssCount = 0; } const sourceDir = "./output/downloads"; diff --git a/custom-css-analysis-and-migration/migrateCssToAppearance.ts b/custom-css-analysis-and-migration/migrateCssToAppearance.ts index 02207d7..8301527 100644 --- a/custom-css-analysis-and-migration/migrateCssToAppearance.ts +++ b/custom-css-analysis-and-migration/migrateCssToAppearance.ts @@ -1,12 +1,13 @@ -/** - * - * I'm not clear what state this is in, it was an experiment I was working on. - * It probably should become a function that is called by create-migrations.ts - * + /** + * function that is called by create-migrations.ts * **/ -import fs from "fs"; +// These are documented at https://www.npmjs.com/package/css/v/3.0.0. import { parse, stringify, Rule, Stylesheet, Declaration } from "css"; +export interface Migration { + modifiedCss: string; + hideL2TitleOnCover: boolean; +} const sizes = [ { @@ -75,55 +76,117 @@ const coverPropertyConversions: PropertyConversions = { width: "--page-margin-right", }; -export function migrateCssToAppearance(cssFileContent: string): string { - var cssObject: Stylesheet; +export function migrateCssToAppearance(cssFileContent: string): Migration { + let cssObject: Stylesheet; + let error: string; try { cssObject = parse(cssFileContent); } catch (e) { - console.log("Error parsing css: " + e); - return cssFileContent; + cssObject = null; + error = e as string; + } + if (error && !cssObject) { + try { + const fixedCss = fixCssIfPossible(cssFileContent, error); + cssObject = parse(fixedCss); + } catch (e) { + console.log("Error parsing css: " + error); + return cssFileContent; + } } + const otherRules: Rule[] = []; + const ruleIndices: number[] = []; + let hideL2TitleOnCover = false; + if (cssObject.stylesheet && cssObject.stylesheet.rules) { - cssObject.stylesheet.rules.forEach((rule: Rule) => { + cssObject.stylesheet.rules.forEach((rule: Rule, index: number) => { + //console.log(`DEBUG rule = ${JSON.stringify(rule)}`); + if ( + rule.type === "rule" && + rule.selectors?.some((s: string) => s.includes(".Title-On-Cover-style[lang=") && + rule.declarations?.some( + (d: Declaration) => d.property === "display" && d.value === "none")) + ) { + hideL2TitleOnCover = true; + } if ( rule.type === "rule" && rule.selectors && rule.selectors.some((s: string) => s.includes(".marginBox")) ) { - // THE Following removal, LIKE A LOT OF THIS FILE, IS GARBAGE. A RULE LIKE - // .marginBox { background-color: red; } is just fine. - - // remove the .marginBox class from the selectors - rule.selectors = rule.selectors.map((s: string) => - s.replace(".marginBox", "") - ); - // find the sizes element that has the same name as one of the selectors const size = sizes.find((sz) => rule.selectors!.some((sel) => sel.includes(sz.name)) ); - var x = rule.declarations?.find( - (d: Declaration) => d.property === "left" - ) as Declaration; - - const l = ( + const leftValue = ( rule.declarations?.find( (d: Declaration) => d.property === "left" ) as Declaration )?.value!; + const left = parseFloatOrUndefined(leftValue); - const t = ( + const topValue = ( rule.declarations?.find( (d: Declaration) => d.property === "top" ) as Declaration )?.value!; + const top = parseFloatOrUndefined(topValue); + + if (!size || !leftValue || !topValue) { + AddWarningCommentsIfNeeded(rule); + return; // leave this rule unchanged. + } + // A rule like .marginBox { background-color: red; } is just fine. + // (A rule like .marginBox .narrowStyle { width: 200px; } is also fine. But there's no reason for + // such a rule to mention .marginBox, so there's no point in improving this code unless we + // encounter a large number of such rules.) + // We're looking for rules that affect the marginBox's size and position (top/left/height/width) + // because the new theme system controls these with variables that affect the .bloom-page margin + // instead of absolutely positioning the .marginBox, so setting its top or left will have no effect, + // and setting its height or width will probably do something unwanted. If such a rule also has + // safe properties, we split it into two rules, one that has only the problem declarations (which + // we try to fix to have the new values needed) and one that has only the safe declarations. + // The .marginBox class is removed from the selectors for the rules with the updated new + // declarations since it's no longer needed. + // Producing split rules also helps human reviewers to see what's going on, and to check that the + // new rules are correct. + const unknownDeclarations = rule.declarations?.filter( + (d: Declaration) => + d.property !== "height" && + d.property !== "width" && + d.property !== "top" && + d.property !== "left" + ); - if (!l || !t) return; // todo log it? + if (unknownDeclarations && unknownDeclarations.length) { + const newRule: Rule = { + type: "rule", + selectors: rule.selectors, + declarations: unknownDeclarations, + }; + sortClassesInSelectors(newRule); + AddWarningCommentsIfNeeded(newRule); + otherRules.push(newRule); + ruleIndices.push(index + 1); + rule.declarations = rule.declarations.filter( + (d: Declaration) => + d.property === "height" || + d.property === "width" || + d.property === "top" || + d.property === "left" + ); + } + + // The remaining declarations in this rule are all safe to keep, but we need to change them + // to use the new variables instead and to change the height and width to bottom and right. + // Also, the .marginBox class is no longer needed in the selector since the new variable + // settings apply to the page outside the .marginBox proper. + rule.selectors = rule.selectors.map((s: string) => + s.replace(".marginBox", "") + ); - var left = parseFloatOrUndefined(l); - var top = parseFloatOrUndefined(t); rule.declarations?.forEach((declaration: Declaration) => { if (declaration.type === "declaration") { const key = (declaration as Declaration).property; @@ -133,11 +196,22 @@ export function migrateCssToAppearance(cssFileContent: string): string { if (v === undefined || left === undefined || top === undefined) declaration.value = ` ignore /* error: ${rule.declarations?.toString()} */`; else { + // Calculate the new value for the declaration from the old value, and round + // off to zero if it's very close to zero. (The new value is either a bottom + // or a right margin instead of a height or width.) Something less than 0.05mm + // is effectively just a rounding error from these floating point subtractions. + // We use val.toFixed(1) since precision greater than 0.1mm isn't worthwhile. + // (Not rounding off to zero explicitly can results in values like -0.0 which + // look odd even if they work okay.) if (key === "width") { - declaration.value = size.width - v - left + "mm"; + let val = size.width - v - left; + if (Math.abs(val) < 0.05) val = 0; + declaration.value = val.toFixed(1) + "mm"; } if (key === "height") { - declaration.value = size.height - v - top + "mm"; + let val = size.height - v - top; + if (Math.abs(val) < 0.05) val = 0; + declaration.value = val.toFixed(1) + "mm"; } } } @@ -145,6 +219,9 @@ export function migrateCssToAppearance(cssFileContent: string): string { const isCover = rule.selectors!.some((sel) => sel.includes("Cover") ); + // Map the existing property name to the new variable name. + // (This takes care of having changed the value from width or height + // to right or bottom above.) const map = isCover ? coverPropertyConversions : propertyConversions; @@ -155,21 +232,21 @@ export function migrateCssToAppearance(cssFileContent: string): string { } }); - // danger, this would probably break if there is anything but classes in the selector - sortClassesInSelector(rule); - - // TODO: this doesn't yet move top and bottom margins with .outsideFrontCover and .outsideBackCover to --cover-margin-top and --cover-margin-bottom - + sortClassesInSelectors(rule); sortDeclarations(rule); } }); - // danger, normally sorting rules is not a good idea! - cssObject.stylesheet.rules = sortRules(cssObject.stylesheet.rules); + if (otherRules && otherRules.length) { + // put the new rules immediately after the rule they were extracted from + for (let i = otherRules.length - 1; i >= 0; --i) { + cssObject.stylesheet.rules.splice(ruleIndices[i], 0, otherRules[i]); + } + } } const modifiedCss: string = stringify(cssObject); - return modifiedCss; + return { modifiedCss, hideL2TitleOnCover }; } function sortDeclarations(rule: Rule) { @@ -180,6 +257,8 @@ function sortDeclarations(rule: Rule) { // Define the property conversions object const orderedProperties: PropertyConversions = { + "--cover-margin-top": 0, + "--cover-margin-bottom": 1, "--page-margin-top": 0, "--page-margin-bottom": 1, "--page-margin-left": 2, @@ -201,32 +280,26 @@ function sortDeclarations(rule: Rule) { } }); } -function sortClassesInSelector(rule: any): void { - // sort the classes in the first selector - const classes = rule.selectors[0].trim().split(".").filter(Boolean).sort(); - const sortedSelector = "." + classes.join("."); - rule.selectors[0] = sortedSelector; -} -function sortRules(rules: Rule[]): Rule[] { - return rules.sort((a: Rule, b: Rule) => { - if (a.type !== "rule" || b.type !== "rule") return 0; - - // sort rules by first selector - const aSelector = a.selectors ? a.selectors[0] : undefined; - const bSelector = b.selectors ? b.selectors[0] : undefined; - if ( - aSelector === bSelector || - aSelector === undefined || - bSelector === undefined - ) { - return 0; - } else if (aSelector > bSelector) { - return 1; - } else { - return -1; +function sortClassesInSelectors(rule: any): void { + // Sort the classes in the selectors if feasible. + // This makes the rules easier to read and to compare. + if (!rule.selectors || !rule.selectors.length) return; + for (let i = 0; i < rule.selectors.length; ++i) { + // Note that rule.selectors is an Array of Strings split on commas. + const selector = rule.selectors[i].trim(); + // If the selector is just a list of classes in the same element, sort them. + // We don't try to sort selectors that have multiple element levels or that + // have something other than classes. + if (/^\.[A-Za-z][-.A-Za-z0-9]*$/.test(selector)) + { + const classes = selector.split(".").filter(Boolean).sort(); + const sortedSelector = "." + classes.join("."); + // We don't need to worry about leading or trailing spaces in the selector + // because the output stringify function has its own ideas about how to format. + rule.selectors[i] = sortedSelector; } - }); + } } function parseFloatOrUndefined(value: string): number | undefined { @@ -237,3 +310,76 @@ function parseFloatOrUndefined(value: string): number | undefined { return undefined; } } + +// This function fixes the input css for the parse errors found in the various CSS +// files that pass the filtering process. The fixes shouldn't affect the semantics +// of the parsed CSS object. +function fixCssIfPossible(cssFileContent: string, error: string) { + let linesOfCss = cssFileContent.split("\r\n"); + if (linesOfCss.length === 1) linesOfCss = cssFileContent.split("\n"); + if (linesOfCss.length === 1) linesOfCss = cssFileContent.split("\r"); + const parsedError = /^Error: [^:]*:([0-9]+):([0-9]+): (.*)$/.exec(error); + if (parsedError && parsedError.length === 4) { + const lineNumber = parseInt(parsedError[1]); + switch (parsedError[3]) { + case "selector missing": // 1 occurrence of a strange contruct + if (linesOfCss[lineNumber - 1].includes("} {")) { + linesOfCss[lineNumber - 1] = linesOfCss[lineNumber - 1].replace( + "} {", + " " + ); + return linesOfCss.join("\r\n"); + } + break; + case "missing '}'": // 2 occurrences of lines missing an obvious '}' + linesOfCss[lineNumber - 1] = "} " + linesOfCss[lineNumber - 1]; + return linesOfCss.join("\r\n"); + case "missing '{'": // 2 occurrences of a '.' on the final line by itself + if (linesOfCss[lineNumber - 1] === ".") { + linesOfCss.splice(lineNumber - 1, 1); + return linesOfCss.join("\r\n"); + } + break; + case "End of comment missing": // 36 occurrences + if (lineNumber === linesOfCss.length) { + return cssFileContent + "*/"; + } + // There's one file with an empty line and a spurious close } by itself on a line + // following the comment that isn't closed properly at the end of the file. + let emptyLines = true; + for (let i = lineNumber; emptyLines && i < linesOfCss.length; ++i) { + if (linesOfCss[i].length >= 2) emptyLines = false; + } + if (emptyLines) return cssFileContent + "*/"; + break; + default: + break; + } + } + return cssFileContent; +} + +function AddWarningCommentsIfNeeded(rule: Rule) { + const comments: Declaration[] = []; + const commentIndices: number[] = []; + rule.declarations?.forEach((d: Declaration, index: number) => { + if (d.property === "bottom" || + d.property === "right" || + d.property?.includes("margin-") || + d.property?.includes("margin:") || + d.property?.includes("padding-") || + d.property?.includes("padding:")) { + const comment: Declaration = {} as Declaration; + comment.type = "comment"; + comment.comment = ` ${d.property}: NOT MIGRATED `; + comments.push(comment); + commentIndices.push(index); + } + }); + if (comments && comments.length) { + for (let i = comments.length - 1; i >= 0; --i) { + rule.declarations!.splice(commentIndices[i], 0, comments[i]); + } + } +} +