Page MenuHomePhabricator

mobile-html: Remove mobile view API dependency when parsoid supports language variants on zhwiki
Closed, ResolvedPublic

Description

Background information

mobile-html is using the mobile view API to enable language variant support on zhwiki

What

Switch zhwiki mobile-html to parsoid when parsoid supports language variants on zhwiki

Event Timeline

LGoto triaged this task as Medium priority.Oct 30 2019, 3:35 PM
LGoto moved this task from Needs triage to Backlog on the Product-Infrastructure-Team-Backlog board.

Change 761782 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/services/mobileapps@master] WIP: Use action=parse API rather than action=mobileview API

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

Rather than using the MobileViewAPi in the mobile content service (T286836#7699314) it seems like it should be possible to use the action=parse API.

The only thing we'd need to work out is how to work around the lack of sections in the parse API response. We could either treat these articles as one single section or maybe you have code inside the transformer which performs section wrapping to the response?

Patch above demonstrates the viability of doing this.

Moving a snippet from a dup'd task into here:

PCS uses the MobileView API for all requests for Chinese Wikipedia (zh.wikipedia.org). From mobile-html.js, line 87 (router.get('/page/mobile-html/:title/:revision?/:tid?', (req, res)):

if (!mobileviewHtml.shouldUseMobileview(req)) {
			return getMobileHtmlFromParsoid(req, res);
		} else {
			return getMobileHtmlFromMobileview(req, res);
		}

and

function shouldUseMobileview(req) {
	return wikiLanguage.getLanguageCode(req.params.domain) === 'zh';
}

hey all, could I get a sense of the timeline here? I'd like to have the MobileView API removed by the end of April 2022. Does this seem like a realistic timeline to get the required updates in the page content service?

We should chat to get on the same page about this. The plan all along was for Parsoid support for zhwiki variants to land much later. So, if we need to reprioritize this, we'll need to figure out reasonable workarounds including introducing whatever MobileView API does into Parsoid or some other such solutions. @cscott is currently on vacation, so we probably have to wait for him to be back March 1. (cc @SLopes-WMF @JMcLeod_WMF)

@ssastry sounds good. I think at this point, I'm simply needing input on how viable https://gerrit.wikimedia.org/r/c/761782 is and what further modifications need to be made. Depending on how that conversation goes, we perhaps need to chat in person.

@ssastry also in case it wasn't clear my proposed solution doesn't rely on Parsoid in any way. It simply swaps out action=mobileview for action=parse https://gerrit.wikimedia.org/r/c/mediawiki/services/mobileapps/+/761782 so all I need here is review for a patch on the existing mobileapp repository.

Sorry I spaced out y'day .. but yes I checked out your patch y'day and looks like you've already done much of the work and are looking for review and feedback on your patch. @cscott is best placed to review this and he is out on vacation and once he is back, we'll go over this. Thanks for doing the legwork!

Change 761782 merged by jenkins-bot:

[mediawiki/services/mobileapps@master] Use action=parse API rather than action=mobileview API

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

I'm back from vacation now -- what remains to be done/reviewed on this task?

@cscott nothing left to review. The change just needs to be deployed and QAed by apps.

I deployed the latest version of mobileapps on beta cluster: https://mobileapps.wmcloud.org/?doc

@cooltey found a couple issues with the patch:

  1. Edit links are appearing incorrectly. We'd expect just an edit pencil, but we're getting it in text on beta cluster:

https://zh.wikipedia.beta.wmflabs.org/api/rest_v1/page/mobile-html/%E8%AB%BA%E6%96%87
https://zh.wikipedia.org/api/rest_v1/page/mobile-html/%E8%AB%BA%E6%96%87

  1. Table of contents is showing up wrong in Android app. While this may be due to different (and poorly structured) content on the beta labs server, you can see that we're getting things like "undefined" in several places. Apps get Table of Contents info by running this JS, and you can do that in web console to replicate: pcs.c1.Page.getTableOfContents()

Change 770032 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/services/mobileapps@master] Tweaks to wrapSections transform

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

Thanks! Let us know when it's live on beta cluster and we can confirm all is well.

Cross-posting here: I have used the mobileapps diff tests to generate fixtures for the zhwiki test comparing your latest patch with the last commit before action=parse was introduced, here is the result https://phabricator.wikimedia.org/transactions/detail/PHID-XACT-PSTE-lecdnh7lt2ppzsx/

It seems to be replacing some section titles h2 with an h6 element and adding an extra h2 tag with class mw-headline

In case you want to reproduce the fixture generation, see npm run test:diff-update at https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/services/mobileapps/+/refs/heads/master/package.json#15

@MSantos thanks for the review, and for the suggested testing approach here. The new patch should address those problems!

Change 770032 merged by jenkins-bot:

[mediawiki/services/mobileapps@master] Tweaks to wrapSections transform

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

Thanks @MSantos for the merge !

@Jgiannelos could we request another deploy so @cooltey can test again? Thanks in advance!

@MattCleinman @cooltey this has been deployed to beta cluster. Can you test and give us a green light to deploy to production?

Hi @Jdlrobson

Just check this page: https://zh.wikipedia.beta.wmflabs.org/api/rest_v1/page/mobile-html/Kotlin

Looks like every section still has the edit link [編輯] on the side of the heading, and also the table of content is not displaying correctly.

Could you please help to check? Thanks!

Looks like every section still has the edit link [編輯] on the side of the heading, and also the table of content is not displaying correctly.

I'm not seeing 編輯 anywhere in the text.

Screen Shot 2022-03-24 at 1.20.30 PM.png (1×1 px, 283 KB)

Just to confirm are we looking at the same page?
(This is the HTML i'm seeing: https://gist.github.com/jdlrobson/3aa1e48808b24146e883aa84dfb220a2)

Yep same URL different results... Do I need to view this in an app? I'm viewing this incognito in Chrome/Firefox. Are you sure this is not an older cached version?

Thanks @Jdlrobson!

I check it on my phone's browser and I can see the latest version. Looks like it is a cached version in the app's Webview.

I wasn't able to clear the cache on my device while viewing it in the app. Is there's a way to purge the cache on the server side so that we can load the latest change? I can recall someone did that before and it worked for me.

I'm not sure I understand what you are asking. If the problem is cache on your device, how would purging the cache on server side help?

Is there another article you could test with?

I was referring to this one: https://phabricator.wikimedia.org/T260684#6475882, which can purge cache in production, but I am not sure you can do it in the beta cluster. Not sure if @MSantos gets it.
und
Other articles were having the same issue which also shows the [編輯] text.

@cooltey the beta cluster doesn't have edge cache AFAIK.

I've purged this page in production https://zh.wikipedia.org/api/rest_v1/page/mobile-html/%E9%98%BF%E6%B3%A2%E7%BD%974%E5%8F%B7 to make sure that no cache is being applied. Could you check it?

Thanks, @MSantos, the page looks good in the app.

The table of the content and also the section headings are looks great in production now. Thanks!

BTW, I am still seeing the wrong version of the mobile-html in the beta cluster, even if I clear all cache or send Cache-control: no-cache header to it.

Sounds good! Does this mean we can proceed with deploying this to production?

Hi @Jdlrobson,

I asked @Sharvaniharan to test it in the app and she also sees the incorrect version of the page.

Screen Shot 2022-03-25 at 11.27.17 AM.png (76×1 px, 18 KB)
image.png (2×1 px, 378 KB)

I am sure that her device does not have a cache version of the beta cluster mobile-html in the app.

What page is this? I don't know enough about apps/api part of the stack to suggest how to fix this but my patch fixed this bug and I don't know how those edit links would appear unless the API cached the result of the previously staged API change.

Presumably an edit to that page should trigger a cache refresh. What page is @Sharvaniharan testing on? Does editing the page make the problem go away?

@Jdlrobson

Not sure which page was @Sharvaniharan testing on, but looks like every page has the same issue.

I tried to edit this page: https://zh.wikipedia.beta.wmflabs.org/wiki/巴姆穆文字 by adding a ~ at the end of the sentence, but the mobile-html does not show the change.

Screenshot 2022-03-25 at 13-03-59 巴姆穆文字 - Wikipedia.png (356×1 px, 62 KB)
desktop
Screenshot 2022-03-25 at 13-04-36 巴姆穆文字.png (1×1 px, 92 KB)
mobile-html

Thanks to @Dbrant who found the issue.

If you add the header Accept-Language: zh-hant when browsing the page, you will see the editing link on the side of each section heading.

1648240739953.jpg (583×1 px, 215 KB)
no header
1648240726415.jpg (646×1 px, 242 KB)
added header Accept-Language: zh-hant

The app will automatically add the Accept-Language for language variants purposes and that's why we always see the editing links in the beta cluster.

I can replicate that now, but only on beta cluster. Can't replicate it locally so this is almost definitely something wrong with the cache:

Screen Shot 2022-03-25 at 2.05.34 PM.png (1×2 px, 555 KB)

Can't replicate it locally so this is almost definitely something wrong with the cache

Just to reiterate what I said on slack.

I made an edit, https://en.wikipedia.beta.wmflabs.org/w/index.php?title=User:Arlolra/sandbox&action=history
and the latest revision isn't being served as the default for the title https://en.wikipedia.beta.wmflabs.org/api/rest_v1/page/html/User:Arlolra%2Fsandbox
but can be retrieved when requested directly https://en.wikipedia.beta.wmflabs.org/api/rest_v1/page/html/User:Arlolra%2Fsandbox/542970

So, yes, it looks to be an issue with RESTBase on the beta cluster. Maybe someone needs to kick ChangeProp there? Someone from Platform Engineering can maybe help.

@cooltey would it be enough for you able to verify this using the revision ID or do we need to get this purged for you to be confident that this can be deployed to production?

Hi @Jdlrobson

I skipped the process of adding the Accept-Language header in the app for the page/mobile-html endpoint, and now I can see the table of content being displayed correctly.

Screenshot_20220325-161240_Wikipedia Dev.jpg (2×1 px, 302 KB)

Since you can test it correctly in the local environment, I am confident that these changes can be deployed to production.

Thanks for your help!

So, yes, it looks to be an issue with RESTBase on the beta cluster. Maybe someone needs to kick ChangeProp there? Someone from Platform Engineering can maybe help.

The issue might have been with eventgate getting stuck on deployment-prep. Not sure, but it's working now

Thanks @Arlolra @cooltey. Looks like the next step is to deploy this to production.

Thanks for the confirmation.
On the 20th March I see 589,351 requests [1] to the mobileview API. 217,011 on 29th.

16,911 hits for zh.wikipedia.org on 29th. On 20th it's 268, 812 [3]
It's quite a big drop, so it probably did get deployed (but it's not zero). Presumably, these are older App clients, which we will tackle as part of T286836.

[1] select count(*) from event.mediawiki_api_request where day = 20 and month = 3 and params['action'] = 'mobileview'
[2] select count(*) from event.mediawiki_api_request where day = 29 and month = 3 and params['action'] = 'mobileview'
[3] select count(*) from event.mediawiki_api_request where day = 20 and month = 3 and params['action'] = 'mobileview' and meta.domain = 'zh.wikipedia.org'