feat: partial port of #381. prefer browser language & safe config.json fetch#440
Merged
DennisOSRM merged 7 commits intogh-pagesfrom May 3, 2026
Merged
Conversation
…nPrecedence: URL param > browser language > runtime config > English\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nPrecedence: URL param > browser language > English\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ig.json when OSRM_ENVIRONMENT='docker' to avoid noisy 404s in non-Docker environments.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tor cases and URL precedence Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DennisOSRM
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR partially ports language-selection and runtime-config loading behavior so the frontend prefers the browser’s language over a hardcoded default, and avoids fetching config.json unless the app is running in the Docker runtime path.
Changes:
- Reworks default language selection in
leaflet_optionsto prefer browser locale candidates and fall back to English. - Replaces the old language override tests with coverage for browser-language matching and URL
hlprecedence. - Adds a Docker-specific signal in
index.htmlanddocker/entrypoint.shsoconfig.jsonis only fetched when the Docker entrypoint prepared runtime config.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/leaflet_options.js |
Adds browser-language detection logic and changes the default language fallback path. |
test/leaflet_options.test.js |
Updates unit tests to cover browser locale selection and URL language precedence. |
index.html |
Adds a meta-based environment check before loading config.json. |
docker/entrypoint.sh |
Rewrites the new meta tag at container startup to indicate Docker runtime mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+250
to
+256
| // exact match (e.g., 'pt-BR') | ||
| if (localization.get(lang)) { | ||
| return lang; | ||
| } | ||
| // primary subtag match (e.g., 'en-US' -> 'en') | ||
| var short = lang.split('-')[0]; | ||
| if (localization.get(short)) { |
Comment on lines
+64
to
+69
| awk -v env="$OSRM_ENVIRONMENT" '{ | ||
| if ($0 ~ /<meta name="osrm-environment"/) { | ||
| sub(/content="[^"]*"/, "content=\"" env "\"") | ||
| } | ||
| }' /usr/share/nginx/html/index.html > "$TMPFILE" && mv "$TMPFILE" /usr/share/nginx/html/index.html || true |
Comment on lines
+201
to
247
| describe('language precedence (URL > browser > en)', () => { | ||
| test('uses browser language exact match', () => { | ||
| global.window = { navigator: { languages: ['de'], language: 'de' } }; | ||
| const leafletOptions = require('../src/leaflet_options'); | ||
| expect(leafletOptions.defaultState.language).toBe('de'); | ||
| delete global.window; | ||
| }); | ||
|
|
||
| test('defaults to en when language not provided', () => { | ||
| global.window = { osrmConfig: {} }; | ||
| test('uses primary subtag when regional locale provided (en-US -> en)', () => { | ||
| global.window = { navigator: { languages: ['en-US'], language: 'en-US' } }; | ||
| const leafletOptions = require('../src/leaflet_options'); | ||
| expect(leafletOptions.defaultState.language).toBe('en'); | ||
| delete global.window; | ||
| }); | ||
|
|
||
| test('prefers first candidate in navigator.languages array', () => { | ||
| global.window = { navigator: { languages: ['fr-CA', 'de'], language: 'fr-CA' } }; | ||
| const leafletOptions = require('../src/leaflet_options'); | ||
| expect(leafletOptions.defaultState.language).toBe('fr'); | ||
| delete global.window; | ||
| }); | ||
|
|
||
| test('matches exact regional variant when available (pt-BR)', () => { | ||
| global.window = { navigator: { languages: ['pt-BR'], language: 'pt-BR' } }; | ||
| const leafletOptions = require('../src/leaflet_options'); | ||
| expect(leafletOptions.defaultState.language).toBe('pt-BR'); | ||
| delete global.window; | ||
| }); | ||
|
|
||
| test('falls back to English when no supported browser languages', () => { | ||
| global.window = { navigator: { languages: ['xx','yy'], language: 'xx' } }; | ||
| const leafletOptions = require('../src/leaflet_options'); | ||
| expect(leafletOptions.defaultState.language).toBe('en'); | ||
| delete global.window; | ||
| }); | ||
|
|
||
| test('URL param (hl) takes precedence over browser default when merged', () => { | ||
| // Simulate browser default 'en' but URL param asks for 'de' | ||
| global.window = { navigator: { languages: ['en'], language: 'en' } }; | ||
| const leafletOptions = require('../src/leaflet_options'); | ||
| const links = require('../src/links'); | ||
| const parsed = links.parse('hl=de'); | ||
| const merged = Object.assign({}, leafletOptions.defaultState, parsed); | ||
| expect(merged.language).toBe('de'); | ||
| delete global.window; | ||
| }); | ||
| }); |
Comment on lines
+202
to
+214
| test('uses browser language exact match', () => { | ||
| global.window = { navigator: { languages: ['de'], language: 'de' } }; | ||
| const leafletOptions = require('../src/leaflet_options'); | ||
| expect(leafletOptions.defaultState.language).toBe('de'); | ||
| delete global.window; | ||
| }); | ||
|
|
||
| test('defaults to en when language not provided', () => { | ||
| global.window = { osrmConfig: {} }; | ||
| test('uses primary subtag when regional locale provided (en-US -> en)', () => { | ||
| global.window = { navigator: { languages: ['en-US'], language: 'en-US' } }; | ||
| const leafletOptions = require('../src/leaflet_options'); | ||
| expect(leafletOptions.defaultState.language).toBe('en'); | ||
| delete global.window; | ||
| }); |
Comment on lines
+235
to
+239
| if (typeof window !== 'undefined' && window.navigator) { | ||
| var nav = window.navigator; | ||
| var candidates = []; | ||
|
|
||
| if (Array.isArray(nav.languages)) { |
…hing, add tests for language detection and entrypoint meta rewrite Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial port of functionality from #381 (large and outdated).
This PR:
Precedence: URL param (hl) > browser default > English.
All tests and linting passed locally.
This is a partial port of #381 — that PR is large and outdated; this extracts the relevant, up-to-date pieces.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com