-
-
Notifications
You must be signed in to change notification settings - Fork 28
Improved checking for totalContentPages reached #8
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ import { | |
| parseJsonpResponse | ||
| } from './utils' | ||
|
|
||
| import * as os from 'os' | ||
|
|
||
| interface PageNav { | ||
| page?: number | ||
| location?: number | ||
|
|
@@ -44,11 +46,23 @@ async function main() { | |
| const krRendererMainImageSelector = '#kr-renderer .kg-full-page-img img' | ||
| const bookReaderUrl = `https://read.amazon.com/?asin=${asin}` | ||
|
|
||
| //Switch for multi-OS operation | ||
| const getChromeExecutablePath = () => { | ||
| switch (os.platform()) { | ||
| case 'win32': | ||
| return 'C:/Program Files/Google/Chrome/Application/chrome.exe'; | ||
| case 'darwin': | ||
| return '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome'; | ||
| default: | ||
| return '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome'; | ||
| } | ||
| } | ||
| const chromePath = getChromeExecutablePath(); | ||
|
|
||
| const context = await chromium.launchPersistentContext(userDataDir, { | ||
| headless: false, | ||
| channel: 'chrome', | ||
| executablePath: | ||
| '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome', | ||
| executablePath: chromePath, | ||
| args: ['--hide-crash-restore-bubble'], | ||
| ignoreDefaultArgs: ['--enable-automation'], | ||
| deviceScaleFactor: 2, | ||
|
|
@@ -259,7 +273,8 @@ async function main() { | |
| if (pageNav?.page === undefined) { | ||
| break | ||
| } | ||
| if (pageNav.page > totalContentPages) { | ||
| if (pageNav.page >= totalContentPages) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed this in one of the books I extracted, but I had to do It's strange because the kindle app calculates the total content pages the same, but you can't navigate to that last page. It then gets stuck in a loop of screenshotting the same page and trying to navigate to the next. With more time, a smarter solution would probably be checking if the current page has already been navigated to, then
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, in my testing it seemed to be slightly different for different books. i never had the chance to improve the robustness, but I like the idea of trying to get the current page ID and breaking if it's been visited There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some books do not include page numbers. I stumbled across this project while doing some research for another, so I decided to see if I could add some robustness to extract-kindkle-book. Seems to work well with three different books I tried, one is a biblical commentary, and the other two were both business books. I hope it helps. |
||
| console.log("Last page reached.") | ||
| break | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldnt merge this in this PR as it's unrelated, and hardcoding paths is not guaranteed, e.g. i do not use chrome and use a custom build of chromium