swift-toolkit icon indicating copy to clipboard operation
swift-toolkit copied to clipboard

Origin is global (app level)

Open JayPanoz opened this issue 7 years ago • 9 comments

After a quick localStorage check, it appears the origin is shared for all publications, and you can retrieve and clear items set in one file from any other file.

It is my understanding it should be possible to scope it to each publication as most iOS Reading Apps I could test are currently doing it.

See Daniel’s doc for further details.

On a funnier note, I could also get all the console.log e.g. scrollToPosition, scrollToOffset, Touch sent to native code. We should probably remove those logs once the app is available in the app store (debug/prod etc.).

JayPanoz avatar Feb 14 '18 12:02 JayPanoz

Hi, following the same logic as Daniel’s doc. It could be solved by WKURLSchemeHandler on iOS 11. But unfortunately, this there is no such API on iOS 10 and older. I am testing other older APIs for possible solution, probably it will require big change in current code.

Here is what I did for iOS 11.

  • Support R2InternalScheme, you can get the R2Internal scheme url from new my function. I didn't change the old function, so you can still the original url. https://github.com/iaomw/r2-shared-swift/commit/f4f223371ad6f7dab6901c0ac54575bebf6f74b0
  • Add R2InternalHandler for our customized WKWebView. It will replace the R2Internal url with original url for any R2Internal request, so it could get the content but still has different origin. https://github.com/iaomw/r2-navigator-swift/commit/05a2238d15455c798fa23c6b422e8f8ab70a81b0

iaomw avatar Mar 22 '18 14:03 iaomw

After my research, I think there are 3 possible ways to solve it on iOS 10 and older:

  1. Download the web page manually with URLSession, then load data with this WKWebView function below. It could have a different url, yet different origin. https://developer.apple.com/documentation/webkit/wkwebview/1415011-load But here is a problem, the WKWebView will download other resource (image, css, js) automatically, there is no API to control resource downloading requests iOS. I found WebResourceLoadDelegate, but it's only available on Mac OS. https://developer.apple.com/documentation/webkit/webresourceloaddelegate
  2. Make separated WKWebsiteDataStore for each WkWebView. I works fine, but Apple only provides the default shared instance and nonPersistent() option. If there is a way to do serialization on the nonPersistent() instance, it will be solved. But it doesn't work. https://developer.apple.com/documentation/webkit/wkwebsitedatastore
  3. In JavaScript, replace the setItem and getItem fucntions on locaStorage. Then call some Swift function inside the new JavaScript functions. As the consequence, I can use any storage solution with Swift. But I am not sure how is the performance. Does it also work with "https"?

iaomw avatar Mar 23 '18 09:03 iaomw

I don't think that these options are viable. IMO, we shouldn't sacrifice everything else just for the sake of solving the Origin issue.

I would defer this issue for now. cc @llemeurfr

HadrienGardeur avatar Mar 23 '18 16:03 HadrienGardeur

We are waiting for other inputs; it is currently put on the side.

llemeurfr avatar Mar 23 '18 16:03 llemeurfr

I looked at iaomw's code, with WKURLSchemeHandler, and I think it's probably the cleanest solution we can implement for iOS 11 and newer. The problem is iOS 11 represents only 65% of devices and iOS 10 is still at 28%.

Regarding the alternatives for older platforms proposed by iaomw, my thoughts are:

  1. I'm not aware of an API provided by Apple or by a third-party library which makes possible to filter data downloaded by type or something similar. I've made some research but didn't find anything. Furthermore, i'm not so enthusiastic by the idea of duplicating data (the original web page, then the downloaded one). I'm not sure, but web page size for an EPUB can be huge, right?

  2. It would be a nice solution, but i'm almost certain it won't be possible to set up. The method nonPersistent() is used to reproduce a private browsing feature so I guess there is an inner mechanism in Apple's Webkit implementation to prevent hijacking browsing history when the user think its navigation is private.

  3. We should be able to implement this one. By intercepting calls to setItem and getItem we could then, in the native Swift code part, rely on something like UserDefaults to safely read and store data. However, it's a lot of overhead. Regarding the performance issue, maybe someone more experienced with JavaScript could answer to the question. We should know how often are used setItem and getItem methods, and we have to find a way to benchmark this alternative implementation of localStorage.

IMO, i would go for the initial solution proposed by iaomw (WKURLSchemeHandler), keeping compatibility with iOS 9 and 10. As llemeurfr said on Slack, risk of data collision could be mitigated by recommending a best practice to authors (publication-specific prefix).

ghost avatar Mar 27 '18 07:03 ghost

Early in this project, we decided that iOS 9.x would be our minimal requirement.

I don't think that we should revisit that decision in this cycle:

  • as it's been pointed out, even on devices that can run iOS 11, many users are still using iOS 10
  • unlike smartphones, tablets have a longer lifecycle and some very popular models of the iPad are currently stuck with iOS 9.x forever

We can re-open that discussion about the minimum version for a future revision cycle, but IMO we absolutely need to deliver a solid 1.0 version that supports iOS 9.x first.

HadrienGardeur avatar Mar 27 '18 08:03 HadrienGardeur

I don't see the point linking the minimal iOS version of the app with the version from which we solve this Origin issue. iOS 9 is the minimal version for the R2 SDK, this will not be re-opened. But we can still decide by consensus that, as Apple does not provide a correct API to handle that issue under iOS 11, we'll live with it on older versions and will alert authors on this fact.

llemeurfr avatar Mar 27 '18 08:03 llemeurfr

For the time-being, I agree with you @llemeurfr but eventually we'll have to release features that won't be compatible with older version of iOS.

I'm mostly thinking about WP/PWP related features, that will probably require (among other things) Service Worker support.

HadrienGardeur avatar Mar 27 '18 12:03 HadrienGardeur

will alert authors on this fact

FWIW, I added that to the Blitz’ Design checklist in February as it’s quite critical. Most Android apps don’t handle the origin as expected anyway – maybe there’s nothing to deal with it on this platform – so yeah, there’s definitely an authoring best practice to promote there.

JayPanoz avatar Mar 31 '18 15:03 JayPanoz