Page_name")
Open, LowPublic
Actions

Assigned To

None

Authored By

	Krinkle
	Apr 17 2020, 5:54 PM

Description

This is follow-up from T250261 and T250205. As part of the unit testing I added in https://gerrit.wikimedia.org/r/589661 I noticed that the purges MediaWiki currently sends for each language variants are completely broken:

For example:

krinkle@mwmaint1002$ mwscript eval.php --wiki zhwiki
var_dump( Title::newMainPage()->getCdnUrls() );
array(
 "https://zh.wikipedia.org/w/index.php?title=Wikipedia:…&zh"
 "https://zh.wikipedia.org/w/index.php?title=Wikipedia:…&zh-hans"
 …
)

Notice the danging language code as a value-less query parameter. That's not something MediaWiki ever uses and also doesn't actually work. These are as meaningless as &rand123 would be, do not actually contain a variant of the page, and most importantly are not the canonical url for that even if it did work - which means it isn't purging the URLs that users would actually be on if they use a particular language variant.

The correct urls, as configured by $wgVariantArticlePath is e.g. /$2/$1, or https://zh.wikipedia.org/zh-hans/Wikipedia:首页 as example.

This brings me to my next point:

$ curl -i 'https://zh.wikipedia.org/zh-sg/Wikipedia:%E9%A6%96%E9%A1%B5' | head -n30
content-language: zh
vary: Accept-Encoding,Cookie,Authorization
expires: Fri, 17 Apr 2020 17:50:36 GMT
cache-control: private, must-revalidate, max-age=0
age: 0
x-cache: cp3064 miss, cp3054 pass
x-cache-status: pass

... page views with a language variant don't have public/CDN caching enabled right now. At least that means the broken purge logic isn't causing stale content.

But it also suggests that the code responsible for caching and the code responsible for purging aren't aware of each other. I suspect the purge code broke at some point, and then a decade later someone noticed there is no purging for it so better disable caching?

For this task:

Remove the dead code.
Decide whether to cache and whether to do it different from normal /wiki/Pagename urls.
Decide whether to purge.
Update Cache-Control logic (where?) to enable caching , if decided so.
Update HtmlCacheUpdater logic to restore language variant logic, if decided so (and this time make it actually work).

Details

	Subject	Repo	Branch	Lines +/-
	title: Remove broken handling of language variant in getCdnUrls()	mediawiki/core	master	+0 -58

Customize query in gerrit

Related Objects

Mentioned In: T279205: Harden and improve HTTP cache headers and purging in MediaWiki core (Sprint placeholder)
T60023: HTMLCacheUpdate should invalidate pages linking to any variant title of the given title if the variant title itself doesn't exist
Mentioned Here: rMW45a3b669cc77: Commiting diff -r 16826:16966 of serbianvariants. Enable engine for aliases…
rMWf684d17b0e66: Don't call Title::getInternalURL with 2 parameters.
T250205: Reduce rate of purges emitted by MediaWiki
T250261: Stop sending purges for `action=history` for linked pages.

Event Timeline

Krinkle created this task.Apr 17 2020, 5:54 PM

Restricted Application added subscribers: Stang, Aklapper. · View Herald TranscriptApr 17 2020, 5:54 PM

Title.php (master)

if ( $this->getPageLanguageConverter()->hasVariants() ) {
    $variants = $this->getPageLanguageConverter()->getVariants();
    foreach ( $variants as $vCode ) {
        $urls[] = $this->getInternalURL( $vCode );
    }
}

This logic was broken in 2016 by commit f684d17b0e66c3, which changed the call from $this->getInternalURL( '', $vCode ); to $this->getInternalURL( $vCode ); because the query2 parameters of getInternalURL and fixUrlQueryArgs were deprecated. But, that also made it no longer use the the legacy behaviour of $query2 being a single-string shortcut for ['variant' => $query2'].

This would be an easy fix, but as mentioned above, these URLs don't need warrant purging right now as they aren't being cached.

After this is removed and after T250261 is fixed, we can think about what we want to do here, and then deal with the fun of injecting LanguageConverter into HtmlCacheUpdater.

@Jdforrester-WMF on Gerrit:

Wow. Added in 45a3b669cc7 on 2006-10-12(!).

Change 589665 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] title: Remove broken handling of language variant in getCdnUrls()

https://gerrit.wikimedia.org/r/589665

gerritbot added a project: Patch-For-Review.Apr 17 2020, 6:08 PM

Krinkle triaged this task as Low priority.Apr 17 2020, 6:36 PM

Jdforrester-WMF subscribed.Apr 17 2020, 7:03 PM

Shizhao added a project: Chinese-Sites.Apr 20 2020, 3:54 AM

Change 589665 merged by jenkins-bot:
[mediawiki/core@master] title: Remove broken handling of language variant in getCdnUrls()

https://gerrit.wikimedia.org/r/589665

Maintenance_bot removed a project: Patch-For-Review.Apr 24 2020, 9:10 PM

Krinkle moved this task from Limbo to Watching on the Performance-Team (Radar) board.May 21 2020, 1:34 AM

Hamishcn moved this task from Backlog to MediaWiki core on the Chinese-Sites board.May 21 2020, 1:55 AM

Hamishcn moved this task from MediaWiki core to Extensions/Skins on the Chinese-Sites board.

Hamishcn subscribed.