From 79fff11335f0faa5261a76be3fe03f3989b19e64 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 26 Mar 2020 15:42:45 -0700 Subject: [PATCH 1/9] android build [nfc]: Keep option next to its argument. With the "--destination" flag next to the unrelated "android" argument before it, but separated by a comment line from its own argument, the formatting of this bit of code obscured its structure. Fix that. Then to help avoid an over-long line, give the argument a name, and pull its definition out from the command line. --- android/app/webviewAssets.gradle | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/android/app/webviewAssets.gradle b/android/app/webviewAssets.gradle index 56bca5d4938..0cb3e9d9fb7 100644 --- a/android/app/webviewAssets.gradle +++ b/android/app/webviewAssets.gradle @@ -66,6 +66,9 @@ gradle.projectsEvaluated { // of what went wrong when it changed out from under us. def assetsDir = "${buildDir}/intermediates/merged_assets/${variant.name}/out" + // See above note on Windows compatibility. + def destDir = normalizePath(relativizePath("${assetsDir}/webview", repoDir)) + def variantTask = tasks.create( name: "build${variant.name.capitalize()}StaticWebviewAssets", type: Exec @@ -74,9 +77,7 @@ gradle.projectsEvaluated { workingDir repoDir executable "bash" args "./tools/build-webview", - "android", "--destination", - // See above note on Windows compatibility. - normalizePath(relativizePath("${assetsDir}/webview", repoDir)) + "android", "--destination", destDir } // Run this task as part of assets merging for this variant. From 18ed7e9f4236b8eaaa6e155d950efcb114a794f8 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 26 Mar 2020 15:52:50 -0700 Subject: [PATCH 2/9] webview build [nfc]: Flip sense of check-dest flag. In a CLI, the defaults generally should favor interactive use, because adding a flag is a much bigger burden on quick command-line use than it is in a script. These checks don't make sense when using this script interactively, so that favors having them opt-in. Moreover, the content of these particular checks is really that they're checking up on whether the caller is doing the caller's own job right -- and doing so on the presumption that the caller is our actual build script for either iOS or Android. (Nothing this tool itself does depends on how the destination directory is named. It'd be pretty weird if it did, and pretty questionable.) The checks are inappropriate in any other context, so that also favors having them be opted into by the specific callers that need them. --- android/app/webviewAssets.gradle | 4 ++-- ios/ZulipMobile.xcodeproj/project.pbxproj | 2 +- tools/build-webview | 15 ++++++++------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/android/app/webviewAssets.gradle b/android/app/webviewAssets.gradle index 0cb3e9d9fb7..fb30edeade1 100644 --- a/android/app/webviewAssets.gradle +++ b/android/app/webviewAssets.gradle @@ -76,8 +76,8 @@ gradle.projectsEvaluated { // All arguments to our script must be relative to `workingDir`. workingDir repoDir executable "bash" - args "./tools/build-webview", - "android", "--destination", destDir + args "./tools/build-webview", "android", "--check-path-name", + "--destination", destDir } // Run this task as part of assets merging for this variant. diff --git a/ios/ZulipMobile.xcodeproj/project.pbxproj b/ios/ZulipMobile.xcodeproj/project.pbxproj index 8784d8695ca..55b3a73d596 100644 --- a/ios/ZulipMobile.xcodeproj/project.pbxproj +++ b/ios/ZulipMobile.xcodeproj/project.pbxproj @@ -1761,7 +1761,7 @@ ); runOnlyForDeploymentPostprocessing = 0; shellPath = /bin/bash; - shellScript = "exec $SRCROOT/../tools/build-webview ios --destination $SRCROOT/webview\n# See this script, or src/webview/static/README.md, for more information.\n"; + shellScript = "exec $SRCROOT/../tools/build-webview ios --check-path-name --destination $SRCROOT/webview\n# See this script, or src/webview/static/README.md, for more information.\n"; }; /* End PBXShellScriptBuildPhase section */ diff --git a/tools/build-webview b/tools/build-webview index 0fb3289aac0..a7b774a9f28 100755 --- a/tools/build-webview +++ b/tools/build-webview @@ -57,7 +57,7 @@ readonly root # Parse arguments. Sloppy, but sufficient for now. unset target unset dest -sanity_checks=1 +check_path_name=0 while (( $# )); do case "$1" in --help|-h|help) @@ -68,9 +68,10 @@ while (( $# )); do --destination) # note: this doesn't permit an equals sign after `--destination` shift; dest="$1"; shift;; - --no-sanity-checks) - # disables target-directory sanity checks, for testing's sake - shift; sanity_checks=0;; + --check-path-name) + # enable persnickety checks of our caller's choice of paths, + # on the assumption it's the actual app build script + shift; check_path_name=1;; *) usage; exit 2;; esac done @@ -86,9 +87,9 @@ fi dest=$(readlink -m "$dest") # Argument parsing has concluded; argument variables are no longer mutable. -readonly target dest sanity_checks +readonly target dest check_path_name -if (( sanity_checks )); then +if (( check_path_name )); then case "$target" in ios) if [ "$(uname)" != "Darwin" ]; then @@ -127,7 +128,7 @@ if (( sanity_checks )); then if [[ "$dest" != *"/webview" ]]; then err "unexpected destination directory '$dest' (expected basename \"webview\")" fi -fi # if $sanity_checks +fi # if $check_path_name # Set $staging. This is our local staging directory; once it's compiled, it will # be synced over to $dest. From b7262129ff5478acf4dc3c1be29a9467f6eb9430 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 26 Mar 2020 16:10:56 -0700 Subject: [PATCH 3/9] webview build [nfc]: Document `--check-path-name` option in usage message. Also describe more plainly what this script does. A long comment in src/webview/MessageList.js uses the term "webview-assets folder", and refers to this file to explain what it means. Deliver on that promise, too, by connecting to that term. --- tools/build-webview | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tools/build-webview b/tools/build-webview index a7b774a9f28..a1a9e2b4333 100755 --- a/tools/build-webview +++ b/tools/build-webview @@ -15,13 +15,18 @@ unset CDPATH # print usage message; do not die usage() { cat >&2 < Date: Mon, 11 Nov 2019 14:15:12 -0800 Subject: [PATCH 4/9] webview build [nfc]: Cut speculative scaffolding comments. We might in the future have complex things we're doing here, which may need a complex structure which may have elements in common with the one suggested by these comments. But at present we're doing something quite simple -- just copying a few files. It isn't productive to try to decide on a complex future structure before we know what concrete needs we'll have for it. Until then, this empty scaffolding just gets in the way of reading what we're actually doing, and of extending it in the directions called for by our current known needs. --- tools/build-webview | 31 +++++-------------------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/tools/build-webview b/tools/build-webview index a1a9e2b4333..5c883b7d098 100755 --- a/tools/build-webview +++ b/tools/build-webview @@ -133,6 +133,11 @@ if (( check_path_name )); then fi fi # if $check_path_name + +################################################################################ +# (Build and) sync +################################################################################ + # Set $staging. This is our local staging directory; once it's compiled, it will # be synced over to $dest. readonly staging="$root/src/webview/static" @@ -141,35 +146,9 @@ if [ ! -d "$staging" ]; then fi -################################################################################ -# Pre-sync: additional build steps -################################################################################ - -# Currently none, but this is where they'll go. -# -# TODO: Use something like `make -f src/webview/static.Makefile`, to ensure that -# complicated build steps are not run unconditionally. -# -# TODO: split assets into build-time-generated and build-time-static? -# - rsync can sync from multiple directories, but has no collision-detection -# - might be too many layers of abstraction - - -################################################################################ -# Sync -################################################################################ - # Sync the directory structure, preserving metadata. (We ignore any files named # 'README.md'; these should be strictly informative.) # # We use `rsync` here, as it will skip unchanged files. mkdir -p "$dest" rsync -a --no-D --delete --delete-excluded --exclude=README.md "$staging/." "$dest" - - - -################################################################################ -# Post-sync: additional build steps -################################################################################ - -# None yet. From 3365154bc849c0edad71b59a045b187df445930b Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 11 Nov 2019 14:02:04 -0800 Subject: [PATCH 5/9] webview build [nfc]: Build straight into destination folder. This "staging" step was intended as a way to help the developer hand-inspect the output for stale files. Better to prevent them automatically, by continuing to use tools like `rsync --delete`. Meanwhile, the extra indirection complicates the code and complicates reasoning about it. So, simplify it away. This change actually has no effect on the current code's behavior, because we're having a source directory double as the "staging" directory, and that's the *only* source directory involved in this build. The real effect comes when we add another source directory: we'll copy from that one straight into the destination, rather than first into this source directory and then again to the destination. This change drops a check that `$root/src/webview/static/` exists. That check is pretty redundant already -- the path is within our own tree -- but in any case `rsync` will give an error if the directory it's supposed to copy from is missing. --- tools/build-webview | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tools/build-webview b/tools/build-webview index 5c883b7d098..271a8a53db7 100755 --- a/tools/build-webview +++ b/tools/build-webview @@ -138,17 +138,11 @@ fi # if $check_path_name # (Build and) sync ################################################################################ -# Set $staging. This is our local staging directory; once it's compiled, it will -# be synced over to $dest. -readonly staging="$root/src/webview/static" -if [ ! -d "$staging" ]; then - err "cannot find asset staging directory '$staging'"; -fi - - # Sync the directory structure, preserving metadata. (We ignore any files named # 'README.md'; these should be strictly informative.) # # We use `rsync` here, as it will skip unchanged files. mkdir -p "$dest" -rsync -a --no-D --delete --delete-excluded --exclude=README.md "$staging/." "$dest" +rsync -a --no-D --delete --delete-excluded \ + --exclude=README.md \ + "$root/src/webview/static/." "$dest" From f68980a3e7efafc36de151e2a26904a545632d34 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 11 Nov 2019 13:49:34 -0800 Subject: [PATCH 6/9] webview build [nfc]: Pull out rsync command into a function. We're about to add another step here, for copying KaTeX-related files out of the KaTeX distribution. Factor out the routine bits into a shell function, so we can invoke the same function for that step too. This also makes a couple of small changes to the rsync command. These don't affect the existing behavior, but they help abstract the shared fiddly-rsync bits, which go inside the function, away from the specific sets of files to copy, which go at the call site. * Use -R aka --relative. This allows a root source directory to be specified once, and all other paths in the command to be relative to that root. * Use rsync filter rules. This allows "which files from the source should be copied" to be specified all in one place, in a well-defined format, and without taking arbitrary rsync arguments which could interact in surprising ways with those provided by the function. --- tools/build-webview | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tools/build-webview b/tools/build-webview index 271a8a53db7..51285b8c1e2 100755 --- a/tools/build-webview +++ b/tools/build-webview @@ -138,11 +138,28 @@ fi # if $check_path_name # (Build and) sync ################################################################################ -# Sync the directory structure, preserving metadata. (We ignore any files named -# 'README.md'; these should be strictly informative.) +# Sync files from src to dest, reading rsync filter rules from stdin. # -# We use `rsync` here, as it will skip unchanged files. -mkdir -p "$dest" -rsync -a --no-D --delete --delete-excluded \ - --exclude=README.md \ - "$root/src/webview/static/." "$dest" +# Both src and dest should be directories. +# +# Files in dest not mentioned will be deleted by default. A rule +# like `-,r /foo` will override this to leave `foo` in place. +# +# For documentation on rsync filter rules, see `man rsync` or: +# https://dyn.manpages.debian.org/testing/rsync/rsync.1.en.html#FILTER_RULES +sync() { + # "_dest" because bash's `readonly` means can't shadow with `local` + local src="$1" _dest="$2" + mkdir -p "${_dest}" + rsync -aR --no-D --delete --delete-excluded \ + "${src}/./" "${_dest}/." \ + --filter='. /dev/stdin' +} + +# Sync the directory structure, preserving metadata. +sync "${root}/src/webview/static" "${dest}" < Date: Thu, 26 Mar 2020 15:36:40 -0700 Subject: [PATCH 7/9] webview build: Whitelist files to copy, rather than blacklist. This has the same effect if the directory is totally clean, containing no files other than those in the Git tree. But if if there are any extraneous files, this prevents accidentally picking them up and building them into the app. Suggested-by: Ray Kraesig --- tools/build-webview | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/build-webview b/tools/build-webview index 51285b8c1e2..8fa0ac8c974 100755 --- a/tools/build-webview +++ b/tools/build-webview @@ -158,8 +158,7 @@ sync() { # Sync the directory structure, preserving metadata. sync "${root}/src/webview/static" "${dest}" < Date: Thu, 26 Mar 2020 14:54:34 -0700 Subject: [PATCH 8/9] webview: Add LaTeX math support! This incorporates the same KaTeX library we use in the Zulip webapp. We add the dependency, add a build-webview step to copy the needed files, and apply the stylesheet in our WebView HTML. There are a couple of cases where the resulting HTML and CSS doesn't quite look right. We'll go on to take care of those in upcoming commits. Fixes: #2660 --- package.json | 1 + src/webview/css/css.js | 1 + tools/build-webview | 21 ++++++++++++++++++++- yarn.lock | 7 +++++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 7f300e2dead..eec2fa1ec4b 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "blueimp-md5": "^2.10.0", "color": "^3.0.0", "date-fns": "^1.29.0", + "katex": "^0.11.1", "lodash.escape": "^4.0.1", "lodash.isequal": "^4.4.0", "lodash.omit": "^4.5.0", diff --git a/src/webview/css/css.js b/src/webview/css/css.js index 2bed42de1f2..0440b418140 100644 --- a/src/webview/css/css.js +++ b/src/webview/css/css.js @@ -6,6 +6,7 @@ import cssNight from './cssNight'; export default (theme: ThemeName) => ` + `; + export default (theme: ThemeName) => ` @@ -17,5 +35,6 @@ ${cssEmojis} display: none; } +${Platform.OS === 'android' ? katexFraclineHackStyle : ''} `;