e884bb61cb
# fix: avoid `topic` fallback for non-Latin titles via pragmatic ASCII transliteration > **Scope update (in response to review):** this PR is intentionally broader than its original "Arabic-only" framing. The implementation changes URL slug generation for **every non-Latin, non-CJK script** that `slugify` previously stripped — see *Scope* below for the explicit list. The goal is *not* linguistically correct romanization; it is "avoid collapsing to `/topic` by producing a usable ASCII slug." ## What this PR is (and isn't) **Goal:** when a question title contains characters outside Basic Latin / Latin Extended / CJK Han, generate a URL slug that is a deterministic ASCII approximation instead of letting `slugify` strip everything and falling back to the literal `"topic"`. **Non-goal:** this is *not* a linguistically correct multi-language romanizer. The output is a machine-acceptable ASCII slug, not what a native speaker would choose. For example, `こんにちは` → `konnichiha` (not the more natural `kon'nichiwa`), `ไทย` → `aithy` (not `thai`). Treat the slug as an opaque, stable, indexable identifier — the path-after-`/questions/<id>/` is for SEO and shareability, the canonical reference is always the ID. ## The bug Pure non-Latin titles previously got stripped by `slugify.Slugify`, hit the empty-result fallback in `htmltext.UrlTitle`, and collapsed to the literal slug `"topic"`. On a live multilingual site, every Arabic / Thai / Japanese-hiragana / Korean / Hebrew / Cyrillic question ended up at `/questions/<id>/topic`. ## The fix `UrlTitle()` gets a `convertNonLatin` pre-step that mirrors the existing `convertChinese` pre-step pattern, using `github.com/mozillazg/go-unidecode` (same author as `go-pinyin` already in the repo, to minimise new-dep friction). ``` UrlTitle(title) → convertChinese(title) // pre-existing: Han-block → pinyin → convertNonLatin(title) // NEW: detect non-Latin letters → unidecode to ASCII → clearEmoji / slugify / url.QueryEscape / cutLongTitle (unchanged) ``` The non-Latin detector skips ASCII, Latin-1 Supplement, Latin Extended-A/B, and CJK Han. Inputs that hit none of those non-Latin letter categories short-circuit and return unchanged, so Latin-only and Chinese-only inputs remain byte-identical (pinned by tests). ## Scope — what scripts are affected This PR changes behavior for **any** title containing letters in scripts that `slugify` doesn't handle. Confirmed by tests in `pkg/htmltext/htmltext_test.go`: | Script | Example title | Before | After | | --- | --- | --- | --- | | Arabic | `كيف حالك` | `topic` | `kyf-hlk` | | Mixed Latin + Arabic | `مرحبا hello` | `hello` | `mrhb-hello` | | Thai | `ไทย ไทย` | `topic` | `aithy-aithy` | | Japanese hiragana | `こんにちは` | `topic` | `konnichiha` | | Korean | `안녕하세요` | `topic` | `annyeonghaseyo` | | Hebrew | `שלום עולם` | `topic` | `shlvm-vlm` | | Cyrillic | `Привет мир` | `topic` | `privet-mir` | **Unchanged:** | Case | Behavior | | --- | --- | | Pure Latin (`hello world`) | unchanged → `hello-world` | | Pure Chinese (`这是一个,标题,title`) | unchanged → `zhe-shi-yi-ge-biao-ti` (pinyin path) | | Japanese with Han-block kanji (`日本`) | unchanged → `ri-ben` (caught by pre-existing pinyin path; treated as Chinese reading, not Japanese — a pre-existing limitation, **not** introduced by this PR) | | Emoji only (`😂😂😂`) | unchanged → `topic` | | Empty / whitespace | unchanged → `topic` | ## Transliteration quality — explicit acknowledgement `go-unidecode` is a generic Unicode → ASCII approximation. It is **not** a per-language romanization library. Specifically: - It will pick *one* approximation per codepoint regardless of language context. `ใ` → `ai` (Thai romanization is `i` or `ai` depending on standard), `한` → `han`, `語` → `Yu` (Chinese pinyin reading even when used in Japanese), etc. - The result is *good enough* to be a stable, URL-safe, human-recognizable handle, but speakers of the source language will not consider it "correct." - It is deterministic, so the same title always produces the same slug — important since `url_title` is recomputed on every request. If maintainers prefer to scope this PR more narrowly (e.g. Arabic only, and reject Thai/Hebrew/Cyrillic/etc.), the detector in `containsNonLatin` can be tightened to specific Unicode blocks — but that means the other scripts continue to collapse to `topic`, which is the bug we're trying to fix. I'd argue the broader fix is preferable to a piecemeal one, but happy to narrow if you want. ## Live deployment / real-world verification This patch has been running in production on **[ask.namasoft.com](https://ask.namasoft.com)** (an Apache Answer instance we operate) since deployment, built directly from this branch via `docker compose build`. The site hosts Arabic-language questions, so the fix exercises the affected code path on every page load. Sample question URL on the deployed instance: > `https://ask.namasoft.com/questions/10010000000000115` The slug in the URL is the transliterated Arabic title rather than `topic`. No data migration was needed since `url_title` is computed on every request from `Title` and never persisted (see *Why this is safe to ship* below). ## Admin-configurable The transliteration is gated by a package-level `atomic.Bool` (default **on**, since the current behavior is objectively broken for affected users): - `htmltext.SetTransliterateNonLatin(enabled bool)` - `htmltext.IsTransliterateNonLatinEnabled() bool` This is deliberately the minimum surface needed to satisfy "the setting must be readable from `UrlTitle()`". A follow-up PR can add an admin UI section that calls `SetTransliterateNonLatin` on save and on startup, without having to re-plumb every `htmltext.UrlTitle` call site through `context.Context`. **Default choice — please confirm:** I picked **default-on** because the existing `topic` behavior is a bug for affected users. If you'd prefer default-off for strict backward compat on existing installs, flip the `init()` in `pkg/htmltext/htmltext.go` to `Store(false)` and surface the toggle as opt-in. ## Why this is safe to ship - `url_title` is **not** a persisted column. It's not on the `Question` entity in `internal/entity/question_entity.go`, no migration has ever added/dropped it, and every call site (`question_service.go`, `revision_service.go`, `vote_service.go`, search/report/review/rank/comment services, controllers, repos) recomputes it from `Title` at response-build time via `htmltext.UrlTitle(...)`. - That means the fix is read-only: existing rows light up with correct slugs on the next request, with no migration and no data rewrite. - Rollback is just redeploying the prior image; nothing on disk changes. ## Test coverage `pkg/htmltext/htmltext_test.go`: - **`TestUrlTitleTable`** — table-driven, one case per affected script (the full matrix above), plus: - `empty` → `topic` - `pure latin unchanged` → byte-identical to pre-fix - `pure chinese unchanged` → byte-identical to pre-fix (pins existing pinyin behavior) - `japanese kanji goes through pinyin path unchanged` → documents the pre-existing Han-block limitation - `emoji only falls back to topic` → unchanged - `long arabic truncates at cutLongTitle boundary` → exercises the 150-byte cap and UTF-8 boundary safety - **`TestUrlTitleTransliterationToggle`** — with the toggle off, non-Latin titles collapse to `topic` (pre-fix behavior); with it on, they transliterate. - Existing `TestUrlTitle` left untouched. Test plan for reviewers: - [ ] `go test ./pkg/htmltext/...` — all pass - [ ] Visit the live sample URL above and confirm slug is transliterated, not `topic` - [ ] Verify Chinese / Latin / emoji-only / empty behavior is byte-identical to `main` (covered by table tests) ## Out of scope (intentionally) - No admin UI / site setting plumbing in this PR — see *Admin-configurable* above. Happy to do the React `Non-Latin Languages Handling` admin page + `SiteType` + service / controller / migration in a follow-up if maintainers want it. - No change to the `"topic"` empty-result fallback. - No plugin interface for slug generation — mirrored the existing `convertChinese` pre-step pattern instead. - No per-language romanization library — this is an explicit non-goal; see *Transliteration quality* above. ## Issues / discussion I didn't find an existing upstream issue covering this — happy to be pointed at one if there is. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: LinkinStars <linkinstar@foxmail.com>
334 lines
9.4 KiB
Go
334 lines
9.4 KiB
Go
/*
|
|
* Licensed to the Apache Software Foundation (ASF) under one
|
|
* or more contributor license agreements. See the NOTICE file
|
|
* distributed with this work for additional information
|
|
* regarding copyright ownership. The ASF licenses this file
|
|
* to you under the Apache License, Version 2.0 (the
|
|
* "License"); you may not use this file except in compliance
|
|
* with the License. You may obtain a copy of the License at
|
|
*
|
|
* http://www.apache.org/licenses/LICENSE-2.0
|
|
*
|
|
* Unless required by applicable law or agreed to in writing,
|
|
* software distributed under the License is distributed on an
|
|
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
|
* KIND, either express or implied. See the License for the
|
|
* specific language governing permissions and limitations
|
|
* under the License.
|
|
*/
|
|
|
|
package htmltext
|
|
|
|
import (
|
|
"fmt"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
)
|
|
|
|
func TestClearText(t *testing.T) {
|
|
var (
|
|
expected,
|
|
clearedText string
|
|
)
|
|
|
|
// test code clear text
|
|
expected = "hello{code...}"
|
|
clearedText = ClearText("<p>hello<pre>var a = \"good\"</pre></p>")
|
|
assert.Equal(t, expected, clearedText)
|
|
|
|
// test link clear text
|
|
expected = "hello [example.com]"
|
|
clearedText = ClearText("<p>hello <a href=\"http://example.com/\">example.com</a></p>")
|
|
assert.Equal(t, expected, clearedText)
|
|
clearedText = ClearText("<p>hello<a href=\"https://example.com/\">example.com</a></p>")
|
|
assert.Equal(t, expected, clearedText)
|
|
|
|
expected = "hello world"
|
|
clearedText = ClearText("<div> hello</div>\n<div>world</div>")
|
|
assert.Equal(t, expected, clearedText)
|
|
}
|
|
|
|
func TestFetchExcerpt(t *testing.T) {
|
|
var (
|
|
expected,
|
|
text string
|
|
)
|
|
|
|
// test english string
|
|
expected = "hello..."
|
|
text = FetchExcerpt("<p>hello world</p>", "...", 5)
|
|
assert.Equal(t, expected, text)
|
|
|
|
// test mixed string
|
|
expected = "hello你好..."
|
|
text = FetchExcerpt("<p>hello你好world</p>", "...", 7)
|
|
assert.Equal(t, expected, text)
|
|
|
|
// test mixed string with emoticon
|
|
expected = "hello你好😂..."
|
|
text = FetchExcerpt("<p>hello你好😂world</p>", "...", 8)
|
|
assert.Equal(t, expected, text)
|
|
|
|
expected = "hello你好"
|
|
text = FetchExcerpt("<p>hello你好</p>", "...", 8)
|
|
assert.Equal(t, expected, text)
|
|
}
|
|
|
|
func TestUrlTitle(t *testing.T) {
|
|
list := []string{
|
|
"hello你好😂...",
|
|
"这是一个,标题,title",
|
|
}
|
|
for _, title := range list {
|
|
formatTitle := UrlTitle(title)
|
|
fmt.Println(formatTitle)
|
|
}
|
|
}
|
|
|
|
func TestUrlTitleTable(t *testing.T) {
|
|
// Long pure-Arabic title: 50 copies of the same Arabic word, joined by spaces.
|
|
// Unidecode of "كيف" is "kyf", so the slug becomes "kyf-" repeated and
|
|
// exceeds cutLongTitle's 150-byte cap.
|
|
longArabic := strings.Repeat("كيف ", 50)
|
|
wantLongArabic := strings.Repeat("kyf-", 37) + "ky" // 37*4 + 2 = 150 bytes
|
|
|
|
cases := []struct {
|
|
name string
|
|
title string
|
|
want string
|
|
}{
|
|
{
|
|
name: "empty",
|
|
title: "",
|
|
want: "topic",
|
|
},
|
|
{
|
|
name: "pure latin unchanged",
|
|
title: "hello world",
|
|
want: "hello-world",
|
|
},
|
|
{
|
|
// Pinyin conversion drops Latin runes by design — matches pre-fix behavior.
|
|
name: "pure chinese unchanged",
|
|
title: "这是一个,标题,title",
|
|
want: "zhe-shi-yi-ge-biao-ti",
|
|
},
|
|
{
|
|
// The fix: previously collapsed to "topic" for all of these scripts.
|
|
// Outputs are an ASCII approximation, not linguistically correct
|
|
// romanization — see PR description.
|
|
name: "arabic transliterated",
|
|
title: "كيف حالك",
|
|
want: "kyf-hlk",
|
|
},
|
|
{
|
|
name: "mixed latin and arabic",
|
|
title: "مرحبا hello",
|
|
want: "mrhb-hello",
|
|
},
|
|
{
|
|
name: "thai transliterated",
|
|
title: "ไทย ไทย",
|
|
want: "aithy-aithy",
|
|
},
|
|
{
|
|
name: "japanese hiragana transliterated",
|
|
title: "こんにちは",
|
|
want: "konnichiha",
|
|
},
|
|
{
|
|
// Japanese with Han-block kanji is caught by the pre-existing pinyin
|
|
// pre-step (Chinese reading, not Japanese), so this path is unchanged
|
|
// by this PR. Pinning to document the existing behavior.
|
|
name: "japanese kanji goes through pinyin path unchanged",
|
|
title: "日本",
|
|
want: "ri-ben",
|
|
},
|
|
{
|
|
name: "korean transliterated",
|
|
title: "안녕하세요",
|
|
want: "annyeonghaseyo",
|
|
},
|
|
{
|
|
name: "hebrew transliterated",
|
|
title: "שלום עולם",
|
|
want: "shlvm-vlm",
|
|
},
|
|
{
|
|
name: "cyrillic transliterated",
|
|
title: "Привет мир",
|
|
want: "privet-mir",
|
|
},
|
|
{
|
|
name: "emoji only falls back to topic",
|
|
title: "😂😂😂",
|
|
want: "topic",
|
|
},
|
|
{
|
|
name: "long arabic truncates at cutLongTitle boundary",
|
|
title: longArabic,
|
|
want: wantLongArabic,
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
got := UrlTitle(tc.title)
|
|
assert.Equal(t, tc.want, got)
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestUrlTitleTransliterationToggle(t *testing.T) {
|
|
defer SetTransliterateNonLatin(true)
|
|
|
|
SetTransliterateNonLatin(false)
|
|
// With transliteration off, pure-Arabic titles collapse to the existing
|
|
// "topic" fallback (the pre-fix behavior).
|
|
assert.Equal(t, "topic", UrlTitle("كيف حالك"))
|
|
|
|
SetTransliterateNonLatin(true)
|
|
assert.Equal(t, "kyf-hlk", UrlTitle("كيف حالك"))
|
|
}
|
|
|
|
func TestFindFirstMatchedWord(t *testing.T) {
|
|
var (
|
|
expectedWord,
|
|
actualWord string
|
|
expectedIndex,
|
|
actualIndex int
|
|
)
|
|
|
|
text := "Hello, I have 中文 and 😂 and I am supposed to work fine."
|
|
|
|
// test find nothing
|
|
expectedWord, expectedIndex = "", 0
|
|
actualWord, actualIndex = findFirstMatchedWord(text, []string{"youcantfindme"})
|
|
assert.Equal(t, expectedWord, actualWord)
|
|
assert.Equal(t, expectedIndex, actualIndex)
|
|
|
|
// test find one word
|
|
expectedWord, expectedIndex = "文", 17
|
|
actualWord, actualIndex = findFirstMatchedWord(text, []string{"文"})
|
|
assert.Equal(t, expectedWord, actualWord)
|
|
assert.Equal(t, expectedIndex, actualIndex)
|
|
|
|
// test find multiple matched words
|
|
expectedWord, expectedIndex = "Hello", 0
|
|
actualWord, actualIndex = findFirstMatchedWord(text, []string{"Hello", "文"})
|
|
assert.Equal(t, expectedWord, actualWord)
|
|
assert.Equal(t, expectedIndex, actualIndex)
|
|
}
|
|
|
|
func TestGetRuneRange(t *testing.T) {
|
|
var (
|
|
expectedBegin,
|
|
expectedEnd,
|
|
actualBegin,
|
|
actualEnd int
|
|
)
|
|
|
|
runeText := []rune("Hello, I have 中文 and 😂.")
|
|
runeLen := len(runeText)
|
|
|
|
// test get range of negative offset and negative limit
|
|
expectedBegin, expectedEnd = 0, 0
|
|
actualBegin, actualEnd = getRuneRange(runeText, -1, -1)
|
|
assert.Equal(t, expectedBegin, actualBegin)
|
|
assert.Equal(t, expectedEnd, actualEnd)
|
|
|
|
// test get range of exceeding offset and exceeding limit
|
|
expectedBegin, expectedEnd = runeLen, runeLen
|
|
actualBegin, actualEnd = getRuneRange(runeText, runeLen+1, runeLen+1)
|
|
assert.Equal(t, expectedBegin, actualBegin)
|
|
assert.Equal(t, expectedEnd, actualEnd)
|
|
|
|
// test get range of normal offset and exceeding limit
|
|
expectedBegin, expectedEnd = 3, runeLen
|
|
actualBegin, actualEnd = getRuneRange(runeText, 3, runeLen)
|
|
assert.Equal(t, expectedBegin, actualBegin)
|
|
assert.Equal(t, expectedEnd, actualEnd)
|
|
|
|
// test get range of normal offset and normal limit
|
|
expectedBegin, expectedEnd = 3, 10
|
|
actualBegin, actualEnd = getRuneRange(runeText, 3, 7)
|
|
assert.Equal(t, expectedBegin, actualBegin)
|
|
assert.Equal(t, expectedEnd, actualEnd)
|
|
}
|
|
|
|
func TestFetchRangedExcerpt(t *testing.T) {
|
|
var (
|
|
expected,
|
|
actual string
|
|
)
|
|
|
|
// test english string
|
|
expected = "hello..."
|
|
actual = FetchRangedExcerpt("<p>hello world</p>", "...", 0, 5)
|
|
assert.Equal(t, expected, actual)
|
|
|
|
// test string with offset
|
|
expected = "...llo你好..."
|
|
actual = FetchRangedExcerpt("<p>hello你好world</p>", "...", 2, 5)
|
|
assert.Equal(t, expected, actual)
|
|
|
|
// test mixed string with emoticon with offset
|
|
expected = "...你好😂..."
|
|
actual = FetchRangedExcerpt("<p>hello你好😂world</p>", "...", 5, 3)
|
|
assert.Equal(t, expected, actual)
|
|
|
|
// test mixed string with offset and exceeding limit
|
|
expected = "...你好😂world"
|
|
actual = FetchRangedExcerpt("<p>hello你好😂world</p>", "...", 5, 100)
|
|
assert.Equal(t, expected, actual)
|
|
}
|
|
|
|
func TestCutLongTitle(t *testing.T) {
|
|
// Short title, no cutting needed
|
|
short := "hello"
|
|
assert.Equal(t, short, cutLongTitle(short))
|
|
|
|
// Exactly max bytes, no cutting needed
|
|
exact150 := strings.Repeat("a", 150)
|
|
assert.Len(t, cutLongTitle(exact150), 150)
|
|
|
|
// Just over max bytes, should be cut
|
|
exact151 := strings.Repeat("a", 151)
|
|
assert.Len(t, cutLongTitle(exact151), 150)
|
|
|
|
// Multi-byte rune at boundary gets removed properly
|
|
asciiPart := strings.Repeat("a", 149) // 149 bytes
|
|
multiByteChar := "中" // 3 bytes - will span bytes 149-151
|
|
title := asciiPart + multiByteChar // 152 bytes total
|
|
|
|
assert.Equal(t, asciiPart, cutLongTitle(title))
|
|
}
|
|
|
|
func TestFetchMatchedExcerpt(t *testing.T) {
|
|
var (
|
|
expected,
|
|
actual string
|
|
)
|
|
|
|
html := "<p>Hello, I have 中文 and 😂 and I am supposed to work fine</p>"
|
|
|
|
// test find nothing
|
|
// it should return from the begin with double trimLength text
|
|
expected = "Hello, I h..."
|
|
actual = FetchMatchedExcerpt(html, []string{"youcantfindme"}, "...", 5)
|
|
assert.Equal(t, expected, actual)
|
|
|
|
// test find the word at the end
|
|
// it should return the word beginning with double trimLenth plus len(word)
|
|
expected = "... work fine"
|
|
actual = FetchMatchedExcerpt(html, []string{"youcant", "fine"}, "...", 3)
|
|
assert.Equal(t, expected, actual)
|
|
|
|
// test find multiple words
|
|
// it should return the first matched word with trimmedText
|
|
expected = "... have 中文 and 😂..."
|
|
actual = FetchMatchedExcerpt(html, []string{"中文", "😂"}, "...", 6)
|
|
assert.Equal(t, expected, actual)
|
|
}
|