Skip to content
This repository has been archived by the owner on Nov 26, 2020. It is now read-only.

Feature/wkwebview #386

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

thilakks
Copy link

@thilakks thilakks commented May 5, 2020

Hi We are looking for our project folio reader with wkWebview

@paulocoutinhox
Copy link
Contributor

Hi @thilakks,

It is working without problems?

Or is WIP?

Thanks.

Copy link
Member

@hebertialmeida hebertialmeida left a comment

Choose a reason for hiding this comment

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

Hey @ricardohg and @thilakks, thanks for contributing.

I have added some suggestions and comments to your PR. This is by far the most asked change by lots of people, thanks for working on that.

Few things I noticed on the behaviour and we need to figure out how to fix that:

  • The "pages left" count is not working properly;
  • The vertical scroll (and horizontal) changes to the next chapter instead of scrolling the content of the chapter, the ideal behaviour is that at the end of the content it should scroll to the next chapter. This probably has to do with the nested scroll views;
  • The base font size is wrong, this is probably some weird WKWebView behaviour and we have to figure out how to reset that to the base size.

Comment on lines +385 to +390
open func js(_ script: String, completion: @escaping JSCallback) {

self.evaluateJavaScript(script) { (result, error) in
completion(result as? String)
}

Copy link
Member

Choose a reason for hiding this comment

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

Making the completion optional will allow you to avoid having to implement it.

Like this.js("resetCurrentSentenceIndex()") instead of .js("resetCurrentSentenceIndex()") { _ in }

Suggested change
open func js(_ script: String, completion: @escaping JSCallback) {
self.evaluateJavaScript(script) { (result, error) in
completion(result as? String)
}
open func js(_ script: String, completion: JSCallback? = nil) {
evaluateJavaScript(script) { (result, error) in
completion?(result as? String)
}
}

Copy link

Choose a reason for hiding this comment

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

sorry can you help me. i add js(_ script: String, completion: JSCallback? = nil) in FolioReaderWebView
Screen Shot 2563-07-22 at 10 40 25

@@ -67,7 +67,7 @@ class FolioReaderAddHighlightNote: UIViewController {

if !highlightSaved && !isEditHighlight {
guard let currentPage = folioReader.readerCenter?.currentPage else { return }
currentPage.webView?.js("removeThisHighlight()")
currentPage.webView?.js("removeThisHighlight()") { _ in }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
currentPage.webView?.js("removeThisHighlight()") { _ in }
currentPage.webView?.js("removeThisHighlight()")

@@ -176,7 +176,7 @@ open class FolioReaderAudioPlayer: NSObject {
@objc func play() {
if book.hasAudio {
guard let currentPage = self.folioReader.readerCenter?.currentPage else { return }
currentPage.webView?.js("playAudio()")
currentPage.webView?.js("playAudio()") { _ in }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
currentPage.webView?.js("playAudio()") { _ in }
currentPage.webView?.js("playAudio()")

@@ -410,7 +415,7 @@ open class FolioReaderAudioPlayer: NSObject {
if synthesizer.isSpeaking {
stopSynthesizer(immediate: false, completion: {
if let currentPage = self.folioReader.readerCenter?.currentPage {
currentPage.webView?.js("resetCurrentSentenceIndex()")
currentPage.webView?.js("resetCurrentSentenceIndex()") { _ in }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
currentPage.webView?.js("resetCurrentSentenceIndex()") { _ in }
currentPage.webView?.js("resetCurrentSentenceIndex()")

@@ -185,7 +185,7 @@ extension FolioReader {

if let readerCenter = self.readerCenter {
UIView.animate(withDuration: 0.6, animations: {
_ = readerCenter.currentPage?.webView?.js("nightMode(\(self.nightMode))")
_ = readerCenter.currentPage?.webView?.js("nightMode(\(self.nightMode))") { _ in }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
_ = readerCenter.currentPage?.webView?.js("nightMode(\(self.nightMode))") { _ in }
_ = readerCenter.currentPage?.webView?.js("nightMode(\(self.nightMode))")


open func webView(_ webView: UIWebView, shouldStartLoadWith request: URLRequest, navigationType: UIWebView.NavigationType) -> Bool {

public func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) {
Copy link
Member

Choose a reason for hiding this comment

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

Please, keep it open as well...

Comment on lines +98 to +105
// let minutesRemaining = Int(ceil(CGFloat((pagesRemaining * totalMinutes)/totalPages)))
// if minutesRemaining > 1 {
// minutesLabel.text = "\(minutesRemaining) " + self.readerConfig.localizedReaderManyMinutes+" ·"
// } else if minutesRemaining == 1 {
// minutesLabel.text = self.readerConfig.localizedReaderOneMinute+" ·"
// } else {
// minutesLabel.text = self.readerConfig.localizedReaderLessThanOneMinute+" ·"
// }
Copy link
Member

Choose a reason for hiding this comment

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

The time left is always 0 or negative, I think you need this calculation.

Comment on lines -374 to +413
paginationMode = .leftToRight
paginationBreakingMode = .page
//paginationMode = .leftToRight
//paginationBreakingMode = .page
Copy link
Member

Choose a reason for hiding this comment

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

Are you planning on fixing this?

Comment on lines +292 to +295
public static func removeFromHTMLById(withinPage page: FolioReaderPage?, highlightId: String, completion: @escaping JSCallback) {
guard let currentPage = page else { return }

currentPage.webView?.js("removeHighlightById('\(highlightId)')", completion: completion)
Copy link
Member

Choose a reason for hiding this comment

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

Same here, if the completion is optional you can ommit it.

Suggested change
public static func removeFromHTMLById(withinPage page: FolioReaderPage?, highlightId: String, completion: @escaping JSCallback) {
guard let currentPage = page else { return }
currentPage.webView?.js("removeHighlightById('\(highlightId)')", completion: completion)
public static func removeFromHTMLById(withinPage page: FolioReaderPage?, highlightId: String, completion: JSCallback? = nil) {
guard let currentPage = page else { return }
currentPage.webView?.js("removeHighlightById('\(highlightId)')", completion: completion)

}

let frameHeight = webView.frame.height
// let lastPageHeight = frameHeight * CGFloat(webView.pageCount) - CGFloat(Double(contentHeight!)!)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this?

@ricardohg
Copy link

Hello @hebertialmeida , thanks for the review!, I need to clarify that we were working in this feature for a specific project, so some stuff we have here are project specific, for instance we only support fixed-layout ebooks, so we did some changes that may break regular epubs. The project is on stand by, so we're not working on this at the moment. however, due to increased interest ( I just saw apple is not accepting UIWebKit anymore) and your code review I think we should end this WKWebView migration. :)

@emirchelsea
Copy link

emirchelsea commented May 7, 2020

Hi all, have you planning in the near future to finish the migration to WKWebview? And also this scrolling issue, how to fix it? I saw on google some suggestions via scroll issue, posted on this blog https://ahmedk92.github.io/2017/11/03/WKWebView-Horizontal-Paging.html

@calebkapil
Copy link

calebkapil commented May 8, 2020

As apple is rejecting apps if they are using the UIWebview, this library won't work till the migration to the WKWebview completed. So put this task on high priority otherwise in spite of being the best library, doesn't sense anything.

@emiraki
Copy link

emiraki commented May 8, 2020

I also find this repo where is the migration to WKWebview. Maybe to you guys who knows more about webviews can help to speed up process of migrations

https://github.com/pmfawkes/FolioReaderKit

@hebertialmeida
Copy link
Member

@ricardohg Yeah, after I posted my review I saw that it was opened by someone using your branch. I lost track of how many people have been asking about this but I haven't had time to work on that, but I can definitely help to review it.

Copy link

@santosh3288 santosh3288 left a comment

Choose a reason for hiding this comment

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

Hello I have migrate and change the UIwebview to WKwebView but fonts and screens not looks good as in UIwebview .. Also I am attaching file of Webkit . Please help me out as I want to upload the app on store .

Screenshot 2020-05-09 at 12 48 18 AM

Copy link

@santosh3288 santosh3288 left a comment

Choose a reason for hiding this comment

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

Hello Can you please help me out I have implemented the same and also I got crashed but I have resolved;lved that now my project is working but I think CSS and js file is not loaded properly in the collection view cell WKwebview , it looks weird can you please let me know how to resolve that issue . here is screen shot
Screenshot 2020-05-09 at 12 48 18 AM

@ricardohg
Copy link

Hello Can you please help me out I have implemented the same and also I got crashed but I have resolved;lved that now my project is working but I think CSS and js file is not loaded properly in the collection view cell WKwebview , it looks weird can you please let me know how to resolve that issue . here is screen shot

Screenshot 2020-05-09 at 12 48 18 AM

Which method are you using to load the html?, it is working on a real device ?

@santosh3288
Copy link

Same method that is mentioned in the file and I only changed UIwebview TO WkwebView

@santosh3288
Copy link

Please help me out its urgent

@emirchelsea
Copy link

Hello everyone, does somebody know how to download epub book, save it to path and load it and then present it in FolioReader? Please help.

@myak0v1ev
Copy link

Hi, everyone.

If you need to fix the issue with font you need do this:
let headerString = "<meta name=\"viewport\" content=\"initial-scale=1.0\" />" webView?.loadHTMLString(headerString + tempHtmlContent, baseURL: baseURL)

@JBenny25
Copy link

@hebertialmeida @ricardohg
Since with WKWebview we can control javascript capability, can we utilize it to add more security to the module?
We can keep javascript disabled for the WebView and temporarily toggle it back while evaluating a script from the code. This way we can be safe from any kind of script injection from the document. Thoughts?

@abhijit-iOS
Copy link

@hebertialmeida What do you think? When will you be able to publish an update with WKWebview? As I am working on one project in which this SDK is the heart of it. So now I am not able to launch that app due to UIWebView. Please do the needful. Thanks.

@grhashmi5
Copy link

It's a great library. Please don't let it die.
Please udpate it the WKWebview :(

@ghost
Copy link

ghost commented May 26, 2020

Just thought of sharing these two forked repos - https://github.com/drasdasdasd/FolioReaderKit
https://github.com/judith-bitcube/FolioReaderKit

The font size issue is solved in this repo. There are minor issues with both of these repos.
but horizontal scroll is flipping through chapters and not pages.

@emirchelsea
Copy link

Since the paginationMode is missing in WkWebview, you need to add this code in your CSS file:

overflow: paged-x;
overflow: -webkit-paged-x;
overflow: -o-paged-x;

@Parsida
Copy link

Parsida commented May 26, 2020

If you're in a rush, consider sponsoring the repository so the contributors can spend more time during these wild times.

@ghost
Copy link

ghost commented May 28, 2020

We have very recently started the work on our iOS app to integrate folioreader. Came across this issue.
@Parsida - how can we sponsor to get this issue resolved soon. If you have any suggestions please let us know.

We did contact @ricardohg to check how we can get his services to get the issue fixed. We are waiting on the response.

@Parsida
Copy link

Parsida commented May 29, 2020

@mooka-ventures Check out @hebertialmeida's page: https://github.com/sponsors/hebertialmeida

@ghost
Copy link

ghost commented May 29, 2020

Thanks, yes we are aware of that page. @Parsida we are already doing a tiny bit of sponsorship starting this month..

@Parsida
Copy link

Parsida commented May 30, 2020

Small donations from several developers that are using this repository in their projects adds up 🙌🏽

@ghost
Copy link

ghost commented Jun 3, 2020

We don't see any other sponsorships to the author @hebertialmeida by other developers.
By the way, we tried to get some freelancer developers to help out with this but I didn't get a positive response from any till now. So we are still waiting for anyone who is ready to complete this issue. Please let us know if anyone is interested in completing this task. We can sponsor the implementation of this specific task.

@hebertialmeida
Copy link
Member

Hey guys, I'm open to keeping code reviewing this but at this time I can't do this implementation. If anyone wants to continue this implementation on another PR feel free to open and I can review.

@OrbitalMan
Copy link

Alright guys. I've managed to fix the scrolling/pagination issue by injecting style tag (scrolling direction dependent) into an html. But I still have to fix the page counting issue and media overlay handling issues. I can post a PR after I will make it work, ok?

@ricardohg
Copy link

Alright guys. I've managed to fix the scrolling/pagination issue by injecting style tag (scrolling direction dependent) into an html. But I still have to fix the page counting issue and media overlay handling issues. I can post a PR after I will make it work, ok?

Hello!, are you working on my branch?, did you check it is working on a real device ?

@OrbitalMan
Copy link

OrbitalMan commented Jun 6, 2020

Hello @ricardohg! Well, I used the most of your code - just edited it according to @hebertialmeida remarks on this PR and fixed the scrolling issue (credits to @emirchelsea). I haven't pushed it anywhere yet. I plan to push it on my fork first. https://github.com/OrbitalMan/FolioReaderKit
The scrolling is confirmed to be working properly on iPhone 7, iOS 13.4.1 BTW.
Upd: I will push the code after I fix the following issues:

  • You have to select a text couple of times in order to have an ability to play the audio from it.
  • The play button from menu also doesn't work before you select the text and make it to show play popup.
  • The pages counter doesn't always calculate correctly.

Also I don't see images embedded in epub content in webview. You can check by opening a book from example project app and looking at empty rect on the first page where the book cover is supposed to be shown. (I don't yet have an idea how to fix it, nor I need to fix it urgently)
Upd2: Ah, I forgot to mention that I also fixed the font scale issue. (credits to @myak0v1ev)

@hebertialmeida
Copy link
Member

@OrbitalMan Thanks for contributing! will you open a new PR then? If so, I can review that and we can go from there.

@OrbitalMan
Copy link

OrbitalMan commented Jun 9, 2020

Sure @hebertialmeida! On the first commit I will make as few changes as possible for your convenience. Later I will refactor the code.

@OrbitalMan
Copy link

OrbitalMan commented Jun 17, 2020

Hey guys, I really think that WebKit/CSS/Javascript approach is overkill (at least for my project requirements - at least for now) - but the problem that migration just screwed up and doesn't work acceptable way. I probably will continue to fix this monster later, some day-ish...
BUT!
I've came up with a native approach solution!

        let textView: UITextView = UITextView()
        let xhtmlURL = Bundle.main.url(forResource: "resource", withExtension: "xhtml")!
        do {
            let xhtml = try String(contentsOf: xhtmlURL)
            let data = Data(xhtml.utf8)
            let attributedText = try NSAttributedString(data: data,
                                                        options: [.documentType: NSAttributedString.DocumentType.html],
                                                        documentAttributes: nil)
            textView.attributedText = attributedText
        } catch {
            print(error)
        }

All you need is just locate your epub's .xhtml files! (you have to unzip the epub - it's up to you how to)
Although this snippet is suitable if you don't need dynamic fonts, etc and if your epub contains single xhtml resource. (and I'm totally fine with that at least for now)

Credits for the snippet go to the Hacking with Swift:
https://www.hackingwithswift.com/example-code/system/how-to-convert-html-to-an-nsattributedstring

@silviocandido
Copy link

@OrbitalMan

Also I don't see images embedded in epub content in webview. You can check by opening a book from example project app and looking at empty rect on the first page where the book cover is supposed to be shown. (I don't yet have an idea how to fix it, nor I need to fix it urgently)

https://developer.apple.com/forums/thread/100125
Maybe it helps! but I don't know how to do it.
Thanks in advance.

@silviocandido
Copy link

@OrbitalMan

Also I don't see images embedded in epub content in webview. You can check by opening a book from example project app and looking at empty rect on the first page where the book cover is supposed to be shown. (I don't yet have an idea how to fix it, nor I need to fix it urgently)
Works for me:
https://github.com/ricardohg/FolioReaderKit/blob/feature/tools/Source/FolioReaderCenter.swift
cell.webView?.loadFileURL(fileURL, allowingReadAccessTo: fileURL.deletingLastPathComponent().deletingLastPathComponent())
cell.loadHTMLString(html, baseURL: fileURL.deletingLastPathComponent())
Thanks to @ricardohg

@mspusta78
Copy link

@OrbitalMan I converted this to SwiftUI and was able to get everything working; however, I see that the latest version of this is still using UIWebView. I used https://github.com/OrbitalMan/FolioReaderKit

Do you by any chance have a repo that uses WKWebview?

I tried https://github.com/drasdasdasd/FolioReaderKit but scrollViewDidEndDecelerating is still using UIWebView.
Commenting that function out did compile and the eBook is looking good. Perhaps we can get that repo merged and updated.

@Supalet
Copy link

Supalet commented Jul 26, 2020

Hello Can you please help me. how to change paginationMode in UIwebview to WKWebview
Screen Shot 2563-07-27 at 02 44 57

@hellbit
Copy link

hellbit commented Sep 29, 2020

Hey guys, who solved a problem with images? Right now WKWebView doesn't load images from epub.

@mspusta78
Copy link

Hey guys, who solved a problem with images? Right now WKWebView doesn't load images from epub.

@hellbit , I ran into the same issue and was able to resolve it by adding loadFileURL

Go to FolioReaderPage.swift and make sure your loadHTMLString function looks like this:

    func loadHTMLString(_ htmlContent: String!, baseURL: URL!) {
        // Insert the stored highlights to the HTML
        let tempHtmlContent = htmlContentWithInsertHighlights(htmlContent)
        // Load the html into the webview
        webView?.alpha = 0
        let headerString = "<meta name=\"viewport\" content=\"initial-scale=1.0\" />"
        let documentDirUrl = try! FileManager.default.url(for: .documentDirectory, in: .userDomainMask, appropriateFor: nil, create: false)
        webView?.loadFileURL(baseURL, allowingReadAccessTo: documentDirUrl)
        webView?.loadHTMLString(headerString + tempHtmlContent, baseURL: baseURL)
    }

@hellbit
Copy link

hellbit commented Oct 4, 2020

@mspusta78 oh, you save my time. )) Thank you, man. I really appreciate it. I will connect with you in Ln.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.