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

[WIP] Merge enhancements and fixes from Readest project into foliate-js upstream #43

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

chrox
Copy link
Contributor

@chrox chrox commented Dec 2, 2024

This PR merges the changes made in the foliate-js submodule on the Readest project as discussed in the Windows build issue of the foliate project. These changes aim to enhance functionality and fix issues encountered during integration. By merging these updates into the upstream foliate-js, future versions of Readest can directly include the foliate-js as a dependency, ensuring alignment with the upstream repository and simplifying maintenance.

Summary of Changes:

1: Avoid the use of Lookbehind regex which is not supported on WebKit version below 614.3.7.1.5;
2: Polyfill CSSStyleSheet constructor which is not supported on WebKit version below 615.1.26.11.22;
3: Avoid the use of module top-level await which is not supported on WebKit version below 613.3.9.1.16;
4: Web Workers loading scripts in pdfjsPath now from the base / that's the public directory of a Next.js project;
5: Use @pdfjs instead of hard coding dist files path of pdfjs so that we can alias @pdfjs to the public directory;
6: A hack to auto-hide the scrollbar by padding in container other than the iframe, so that container could grab the mouseleave event and hide the scrollbar on macOS, otherwise once the scrollbar is shown it will be visible forever which is quite annoying on macOS;
7: To implement Parallel View in Readest, the scrollTo function of the paginator should be public.

Additional Notes:

Let me know if there are any adjustments or further refinements required for compatibility or style. Thank you for reviewing this patch! Looking forward to discussing and refining these updates for inclusion in foliate-js.

@johnfactotum
Copy link
Owner

johnfactotum commented Dec 2, 2024

1, 2, & 3: As hinted in the readme, I do not wish to support older browsers. In particular, it seems these features are supported if you're using the oldest currently supported version of macOS/iOS.

4 & 5: The reason these are the way they are now is that foliate-js is designed to be imported directly as is, as native ES modules. I'm not sure how it would work if you make these changes.

6: It's not entirely clear to me what the issue is, but I presume one can use the mouseleave event on the document in the iframe?

7: I have not tried the feature so I don't know how it works exactly. If you just need to scroll to a certain fraction of a section, you can use a number as the anchor. E.g. view.renderer.goTo({ index: 0, anchor: 0.5 }) will scroll 50% through the first section. This has the advantage of working also in paginated mode.

@chrox
Copy link
Contributor Author

chrox commented Dec 2, 2024

1, 2, & 3: To support these features the app needs to target macOS 13.3 (which is release early last year) and above. I think it's fine to drop supports of lower versions.

4 & 5: The reason I need this is also supporting older browsers so that only legacy build of pdfjs is used. So if supports of older browsers are dropped this is fine.

6: To auto-hide the scrollbar the container needs to be notified the mouseleave events. When there is no padding between the container and the iframe it seems no mouseleave events are detected in the container. So this is really a dirty hack.

7: That will be great. I will try this API now.

@chrox
Copy link
Contributor Author

chrox commented Dec 2, 2024

Great. 6 & 7 are fixed in Readest side. Only 1 - 5 are needed to support older browsers. I'll maintain these patches for now and squash all future compatibility patches into this single commit.

@johnfactotum
Copy link
Owner

According to caniuse.com, those features are supported since Safari 16.4, and according to https://developer.apple.com/documentation/safari-release-notes the latest version of Safari available for macOS 11 Big Sur is 16.6. The oldest version of macOS still getting security updates, from what I gather, appears to be 12 Monterey. Am I missing something?

Another thing that occurred to me is CSS nesting. Are you polyfilling for that?

@chrox
Copy link
Contributor Author

chrox commented Dec 3, 2024

Another thing that occurred to me is CSS nesting. Are you polyfilling for that?

I have no idea of this and I didn't polyfill it. And on older browsers like AppleWebKit/613.3.9 pdf rendering is unusable, the text layer is visible and misaligned with the pdf page layer for both text-based pdfs and image-based pdfs. I don't know if it's related to CSS nesting and I haven't had time yet to investigate into this. Btw AppleWebKit/615.3 and above don't have this issue.

image

image

UPDATE: This issue is already fixed by readest/readest@0076bab.

@chrox
Copy link
Contributor Author

chrox commented Dec 3, 2024

According to caniuse.com, those features are supported since Safari 16.4, and according to https://developer.apple.com/documentation/safari-release-notes the latest version of Safari available for macOS 11 Big Sur is 16.6. The oldest version of macOS still getting security updates, from what I gather, appears to be 12 Monterey. Am I missing something?

One of my MacBook(MacBook Pro 13-inch, 2015) is stuck at macOS 12.7.6 as Apple has discontinued to support this model. Even though the last update of macOS 12.7.6 brings the Safari to version 17.6 but according to Tauri's documentation:

On macOS, Tauri uses the webview that comes preinstalled with macOS since version 10.10 (Yosemite). It is considered a core component and is therefore updated with the regular OS updates. This means unsupported macOS versions do not receive WebKit updates.

It is verified with this:

~ awk '/CFBundleVersion/{getline;gsub(/<[^>]*>/,"");print}' /System/Library/Frameworks/WebKit.framework/Resources/Info.plist

	17613.3.9.1.16

Even it has a Safari with Version 17.6 (17618.3.11.11.7, 17618), the preinstalled WebKit with the operating system is stuck at 17613.3.9.1.16. So if we target to macOS version 13.3 and above which has WebKit version 17615.1.26, almost all MacBook before 2017 are ruled out.

@chrox
Copy link
Contributor Author

chrox commented Dec 3, 2024

Another thing that occurred to me is CSS nesting. Are you polyfilling for that?

I see it. The annotation_layer_builder.css and text_layer_builder.css use a lot of CSS nesting. After they are flattened the PDF rendering is perfect on my old MacBook. 👍

1. Avoid the use of Lookbehind regex which is not supported on WebKit version below 614.3.7.1.5;
2. Polyfill CSSStyleSheet constructor which is not supported on WebKit version below 615.1.26.11.22;
3. Avoid the use of module top-level await which is not supported on WebKit version below 613.3.9.1.16;
4. Use legacy build of pdfjs instead of the bundled pdfjs in the vendor directory:
  * Web Workers loading scripts in pdfjsPath now from the base / that's the public directory of a Next.js project;
  * Use @pdfjs instead of hard coding dist files path of pdfjs so that we can alias @pdfjs to the public directory;
@chrox chrox force-pushed the main branch 3 times, most recently from 6774111 to c7b4475 Compare January 9, 2025 00:56
in order to support more TTS backends
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