fix: Start to incorporate lang in URL paths 🌋#704
fix: Start to incorporate lang in URL paths 🌋#704darcywong00 wants to merge 39 commits intoepic/i18n-url-schemefrom
Conversation
User Test ResultsTest specification and instructions User tests are not required |
* These folders don't use _content
1. remove .php / .md 2. redirects before rewrites 3. rewrite locale to lang= parameter 4. final cleanup
7a87423 to
bf9897e
Compare
also cleaned up test pages
| @@ -0,0 +1,4 @@ | |||
| <?php | |||
| // Redirect to Android homepage | |||
| header('Location: https://help.keyman.com/products/android/'); | |||
There was a problem hiding this comment.
Converted from /_content/android/app/.htaccess - also done for iphone-and-ipad/app
Root .htaccess rules weren't matching
There was a problem hiding this comment.
I'm a little uncomfortable with this change -- it might be better to add the rule at the top level .htaccess?
There was a problem hiding this comment.
ok, will convert this too top level .htaccess rule(s)
| </div> | ||
| <div class="download-cta-small iPad" id="cta-iPad"> | ||
| <a href="../../../ipad/"> | ||
| <a href="../../../../ipad/"> |
| </p> | ||
| <p> | ||
| <span class="red">A.</span> Yes, you can find the links to download Keyman Desktop 7.1 and other previous releases <a href="../archive/downloads.php">here</a>. | ||
| <span class="red">A.</span> Yes, you can find the links to download Keyman Desktop 7.1 and other previous releases <a href="/archive/downloads">here</a>. |
There was a problem hiding this comment.
| <span class="red">A.</span> Yes, you can find the links to download Keyman Desktop 7.1 and other previous releases <a href="/archive/downloads">here</a>. | |
| <span class="red">A.</span> Yes, you can find the links to download Keyman Desktop 7.1 and other previous releases <a href="../archive/downloads">here</a>. |
Why make the path absolute again?
There was a problem hiding this comment.
This depends on the root level redirect
Lines 64 to 65 in 11c3a70
When I make it relative path, I get a 404
Since this is our own link, maybe I should just fix it to ../downloads/archive
There was a problem hiding this comment.
Definitely we should fix the link on our site, no point in additional complexity and redirects.
But this highlights an upcoming issue with redirects which I think we kinda decided punt until later: what do we do with things like keyman.com/khmer --> /keyboards/khmer_angkor? Do we also support keyman.com/en/khmer --> /en/keyboards/khmer_angkor? Do we do this for all redirects or a subset or none?
I am unsure.
| // Fallback to 'en' | ||
| // TODO-I18N-URL-SCHEME: integrate with Locale.php | ||
| $fields->lang = isset($_REQUEST['lang']) ? $_REQUEST['lang'] : 'en'; |
There was a problem hiding this comment.
Why remove this? We'll need something like this throughout
There was a problem hiding this comment.
ah, I got mixed up thinking the new .htaccess rules would handle the language tag.
So we want to keep $fields->lang for Foot, Menu?
There was a problem hiding this comment.
Yes, we'll want to keep those fields but they'll need to be extracted from the URL. I think we'll be able to parse the URL as it should be the first path component, rather than using URL rewriting to append a lang parameter?
| ## Links to Other Test Pages | ||
|
|
||
| [/go links](./go) | ||
| [/go links](/_test/go) |
There was a problem hiding this comment.
The CI link checker test was failing
https://github.com/keymanapp/keyman.com/actions/runs/24058950224/job/70171010949#step:7:18
I don't quite understand it, but the CI link checker would treat these links as:
Getting links from: http://localhost:8053/_test
├─BROKEN─ http://localhost:8053/go (HTTP_403)
├─BROKEN─ http://localhost:8053/language-landing (HTTP_404)
├─BROKEN─ http://localhost:8053/legacy (HTTP_404)
├─BROKEN─ http://localhost:8053/well-known (HTTP_404)
Error: Process completed with exit code 1.There was a problem hiding this comment.
This means that the rule which appends a / in a redirect for index.md is missing or out of order; we'll need that.
- /_test should redirect to /_test/ because it is a folder.
alongside the rules that strip .php and .md suffix and deal with index filename:
- /_test/index.php -> /_test/index
- /_test/index.md -> /_test/index
- /_test/index -> /_test/
There was a problem hiding this comment.
k, still troubleshooting this one.
Will get back to this after Songkran break!
There was a problem hiding this comment.
aha, {DOCUMENT_ROOT} needed to be %{DOCUMENT_ROOT}
fixed in 88769e9 along with
- cleaning up links in _test
- cleaning up unncessary
\/to/in .htaccess
Co-authored-by: Marc Durdin <marc@durdin.net>
| <a href="/<?=$fields->lang?>/about/"><img id="sil-logo" src="<?php echo ImageRandomizer::randomizer(); ?>" width="50%" alt='SIL' /></a> | ||
| <p>Created by <a href="/<?=$fields->lang?>/about/">SIL Global</a></p> | ||
| <a href="/about/"><img id="sil-logo" src="<?php echo ImageRandomizer::randomizer(); ?>" width="50%" alt='SIL' /></a> | ||
| <p>Created by <a href="/about/">SIL Global</a></p> |
There was a problem hiding this comment.
btw, why doesn't about SIL Global go to https://global.sil.org/about/ ?
| </p> | ||
| <p> | ||
| <span class="red">A.</span> Yes, you can find the links to download Keyman Desktop 7.1 and other previous releases <a href="../archive/downloads.php">here</a>. | ||
| <span class="red">A.</span> Yes, you can find the links to download Keyman Desktop 7.1 and other previous releases <a href="../downloads/archive">here</a>. |
There was a problem hiding this comment.
This link is cleaned up
Follows #696
This overhauls .htaccess files to handle a langtag in the URL path (rewrite to corresponding _content/ path).
Flow of .htaccess:
/fileto<lang>/fileAdditional small changes:
_content/archive/.htaccessrule to the root .htaccessCI link checker
willmay still failTest-bot: skip