Merge Changes from NOOK Client #438
Conversation
…Pad Pro 12.9 media queries
…6 Plus/iPad Pro 12.9 media queries" This reverts commit af07072.
…ize by Javascript
…ew and index when PAGINATION_CHANGED/FXL_VIEW_RESIZED
…oid client The reason is Chrome mobile would disable setInterval/setTimeout inside event handler, then cause iBooks audio would never been playback after user taps on iBooks element
| @@ -1,3 +1,3 @@ | |||
| [submodule "readium-cfi-js"] | |||
| path = readium-cfi-js | |||
| url = https://github.com/readium/readium-cfi-js.git | |||
| url = http://stash.barnesandnoble.com/scm/ios/clients-ios-externals-readium-cfi-js.git | |||
There was a problem hiding this comment.
Submodule references pointing to private vendor instances should definitely not be merged into the open-source GitHub-hosted codebase ;)
| grgit.push(tags:true) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This seems very specific to barnesandnoble.com, and it is Android-related, right? (not generic readium-shared-js code)
| position: absolute; | ||
| top: 50%; | ||
| right: 5%; | ||
| } |
There was a problem hiding this comment.
I think this CSS is app-specific, not in the scope of readium-shared-js.
| cd "${pwd}" | ||
|
|
||
| echo "###########################################" | ||
| echo "###########################################" |
There was a problem hiding this comment.
I think this was part of the BN native app, not meant to be in readium-shared-js
| eval splitJvmOpts $DEFAULT_JVM_OPTS $JAVA_OPTS $GRADLE_OPTS | ||
| JVM_OPTS[${#JVM_OPTS[*]}]="-Dorg.gradle.appname=$APP_BASE_NAME" | ||
|
|
||
| exec "$JAVACMD" "${JVM_OPTS[@]}" -classpath "$CLASSPATH" org.gradle.wrapper.GradleWrapperMain "$@" |
There was a problem hiding this comment.
These Gradle files should not be part of this PR, I think (vendor-specific com.nook.core.readium, for example)
| } | ||
| node.clipEnd = node.MAX; | ||
| node.clipEnd = node.clipBegin + 0.1; | ||
| } |
There was a problem hiding this comment.
- due to this code change, if
clipBegin==clipEndthen the audio player will probably misbehave, right? - what is the rationale for this change? Before the change, setting
clipEndto MAX (whenclipEnd<=clipBegin... granted, that's a weird edge-case) informs the audio player keep playback until the natural end of the audio file. With the change, the audio player will play 0.1 seconds of content?
| @@ -603,7 +633,7 @@ define(['jquery'],function($) { | |||
| } | |||
|
|
|||
| var MAX_SEEK_RETRIES = 10; | |||
| var _seekedEvent1 = _iOS ? "canplaythrough" : "seeked"; //"progress" | |||
| var _seekedEvent1 = "seeked"; | |||
There was a problem hiding this comment.
This reminds me of this bug report (Android Chromium webview breaking changes):
readium/readium-js-viewer#681
...I really hope that one day no audio-related code tweaks are needed for modern iOS (thank you BN for the PR, it looks like based on your experience the "seeked" event now works), but on Android the practical reality is that Readium must support older version (e.g. KitKat 4.4) where some tweaks are still required :(
| @@ -1214,7 +1214,7 @@ var CfiNavigationLogic = function (options) { | |||
| if (!firstPartial) firstPartial = item; | |||
|
|
|||
| if (visible == 100) return item; | |||
| } | |||
| } else return item; | |||
There was a problem hiding this comment.
What is the reason for this change? This seems to run counter to finding the first visible Media Overlays element. Without this code change, this is equivalent to continue (with respect to the enclosing loop), but with the change the first non-visible MO element will be found!
|
|
||
| if (!spineItem.media_overlay_id && mediaOverlay.smil_models.length === 0) { | ||
| if ($body.length > 0) { | ||
| Helpers.addTapEventHandler($body[0], tapEmitter); | ||
| } | ||
| return; |
There was a problem hiding this comment.
Thank you for this contribution. Touch handling improvements in the Media Overlay implementation are welcome indeed.
| @@ -0,0 +1,3 @@ | |||
| [submodule "src/fabric"] | |||
| path = src/fabric | |||
| url = https://github.com/kangax/fabric.js.git | |||
There was a problem hiding this comment.
html2canvas seems to be referencing a submodule (kangax/fabric) that does not exist in this source tree. Is that intentional? Or should the external_modules folder be entirely deleted from this PR?
These are changes from NOOK client for ePub3 reader.