-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
fix: Modify stable url for keyboard download to kmp file #475
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is this a much older bug, not actually coming out of the Apache/docker transition? Let's make sure we get it right for all patterns. The bundled installer .exe is now actually not a "bundle" but a file name pattern which causes the bundle to download (see https://kmn.sh/kb35, keyman-setup filename patterns section), so we may need to do some more work to get that right.
@@ -8,7 +8,8 @@ RedirectMatch "/go/(([1-9][0-9])([.]?)([0-9]))/developer-help-(mobile|packages)( | |||
# Download redirects for keyboard permalinks (TODO: these three rules need refresh) | |||
|
|||
# download-kmp | |||
RedirectMatch "/go/keyboard/([^/?]+)/download/kmp$" "/keyboards/download?id=$1&platform=windows&mode=standalone" | |||
# Modified to match download-package below | |||
RewriteRule "^keyboard/([^/]+)/download/kmp$" "package/download.php?type=keyboard&id=$1" [END,QSA] | |||
|
|||
#download-exe | |||
RedirectMatch "/go/keyboard/([^/?]+)/download/exe$" "/keyboards/download?id=$1&platform=windows&mode=bundle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this rule and download-js? They also seem to be wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For .exe, look at the pattern generated e.g. by https://keyman.com/keyboards/install/khmer_angkor, and use that: https://downloads.keyman.com/windows/stable/17.0.331/keyman-setup.khmer_angkor.exe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
download-js should go to downloads.keyman.com/keyboards/name/version/name.js, e.g. https://downloads.keyman.com/keyboards/thamizha_bamini/2.2/thamizha_bamini.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Splitting download-exe and download-js to be handled in #495
Fixes #474
From the reported issue,
We want to change /go/keyboard/**/download/kmp to match the behavior of /go/package instead of going to a keyboard landing page.
This modifies the rule in .htaccess to match what /go/package does.
keyman.com/go/.htaccess
Lines 27 to 28 in b695ba0
The links can be manually compared on
keyman.com/_test/go.md
Lines 348 to 354 in b695ba0
Until this PR is merged, localhost will redirect to the kmp file while the live site still goes to the keyboard page.