Add support for language variant filter #1005
Conversation
Pchelolo
force-pushed
the
siteinfo_langvariants
branch
2 times, most recently
from
09f3fda
to
12862d2
Compare
Change-Id: Ia7fbafe8abc73e96b391e77176d3873be962471b
Pchelolo
force-pushed
the
siteinfo_langvariants
branch
from
12862d2
to
93c8d9a
Compare
d00rman
changed the title
lib/mwUtil.js
Outdated
| return false; | ||
| } | ||
| // Second, if the requested language variant exist for page language | ||
| return pageLangVariants.indexOf(acceptLanguage) !== -1; |
There was a problem hiding this comment.
It would be good if the check were case-insensitive.
| if (/\/page\/html\//.test(specInfo.path)) { | ||
| // It's HTML, hit Parsoid for conversion | ||
| return next(hyper, req) | ||
| .then(html => hyper.post({ |
There was a problem hiding this comment.
All this block looks like it belongs in the Parsoid module rather than here since we keep all Parsoid-related interfaces and logic there.
There was a problem hiding this comment.
It's basically just making a request to the parsoid module here. We can probably move a bit more logic in there, but not too much..
lib/language_variants_filter.js
Outdated
| // We can skip setting Vary: accept-language as Parsoid sets it. | ||
| } else { | ||
| // TODO: It's something else, so just forward to MCS | ||
| return next(hyper, req); |
There was a problem hiding this comment.
Right now this will also store the result while forwarding the A-L header, won't it?
sys/action.js
Outdated
| Object.keys(origVariants).forEach((lang) => { | ||
| variants[lang] = Object.keys(origVariants[lang]) | ||
| // Filter out non-specific variants like `en`, `zh` etc. | ||
| .filter(variant => /-/.test(variant)); |
There was a problem hiding this comment.
What about multi-lang wikis? Wouldn't en be a valid "variant" for commons, e.g.?
There was a problem hiding this comment.
In case of commons/wikidata it's all pretty different - this is just about conversion, commons actually would return a completely different response. I don't think parsoid even supports that case.
| @@ -735,6 +735,7 @@ class ParsoidService { | |||
| headers: { | |||
| 'content-type': 'application/json', | |||
| 'user-agent': req['user-agent'], | |||
| 'content-language': req.headers['content-language'] | |||
There was a problem hiding this comment.
Did you mean accept-language ?
There was a problem hiding this comment.
Nope, I meant content-language - parsoid requires that header to be set for the conversion request.
Change-Id: Icf42fca39e9691ecf92d39017f63482e0d0716f1
Change-Id: I3c22de412c8735021daf97db465cd6455c57e33f
Change-Id: Id0d6d2868c9cef5f6526d9129ff0f12b165a1f7e
Change-Id: Icc9818b299cb0f5233529cdb15e98510bd550abc
WIP: Pending actual parsoid transformations support.
cc @wikimedia/services