Page MenuHomePhabricator

mobile-html: ability to preview an edited page or section with the same transforms and styles as mobile-html
Closed, ResolvedPublic

Description

Background information

Apps currently use the mediawiki api with action=parse and mobileformat=true to transform user-generated wikitext into a previewable page. Ideally, there'd be an endpoint that returns the user-generated wikitext as a page with formatting that matches the new article viewing endpoint, mobile-html.

How

Either an endpoint where we can POST Parsoid HTML for transformation into a mobile-html page

-OR-

Something along the lines of:
pagelib.c1.PageMods.replaceContent(contentHtml)
pagelib.c1.PageMods.replaceSection(id, sectionHtml)

Event Timeline

JoeWalsh created this task.Dec 3 2018, 3:54 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 3 2018, 3:54 PM
bearND renamed this task from [FEATURE REQUEST] Endpoint that renders user edits in the same format as mobile-html to [FEATURE REQUEST] Endpoint that renders user edit previews in the same format as mobile-html.Dec 3 2018, 4:06 PM

I wonder if we should do something similar to https://en.wikipedia.org/api/rest_v1/#!/Transforms/post_transform_wikitext_to_html_title_revision and use the same as the backend service, just run the transformations on top of it.

@Joe Do you only need one single section in there + a preview of references defined in that section?
//cc @ssastry

@bearND after getting clarification of the product plans, it'd be ideal if we could either preview a single section or the entire article. Our main use case will be editing single sections, but there will also be the possibility of editing the entire article.

@bearND @JoeWalsh note that there is something to be aware of here with previewing a section. If a section references a named ref that is defined elsewhere in the article, when you submit that wikitext for preview, that named ref will be flagged as being undefined (same as the wikitext editor which says "Cite warning: <ref> tag with name SOME_NAME_HERE cannot be previewed because it is defined outside the current section or not defined in this article at all."). So, if you need behavior that is different than that, it gets complicated.

Also, it's gonna be interesting to implement this. MCS output is based on Parsoid HTML, and the HTML is not POSTed to MCS, it GETs it from storage. So in order to implement the transform endpoint, we will need MCS to support a completely different request flow. Not that it is impossible, it just might be a bit more work than expected.

bearND added a comment.Dec 3 2018, 9:39 PM

@Pchelolo Yeah, that's why I think this new endpoint would probably be similar to /transform/wikitext/to/html{/title}{/revision}.

@ssastry Isn't the above endpoint what VE uses? Or are they using /transform/section-changes/to/wikitext/{title}/{revision} instead?

@ssastry Isn't the above endpoint what VE uses?

VE doesn't need to use that -- it GETs the stored HTML.

But, afaik, the 2017 wikitext editor hits that endpoint.

@Pchelolo Yeah, that's why I think this new endpoint would probably be similar to /transform/wikitext/to/html{/title}{/revision}.

Externally - yes. but in order to process the POSTed content, we would need to first hit Parsoid to produce HTML (we have means of doing that) and then hit MCS to reparse the HTML into mobile oputput. Currently MCS is not capable of taking in HTML and we do now want to temporary store that. That's what I mean saying we would need some more work to be done on MCS side.

@Pcheclolo I was thinking the new PCS endpoint would call the corresponding Parsoid endpoint (internally).

@bearND Currently MCS (PCS) only talks to MW and RB. Adding Parsoid into the mix would not be my preferred way to go. Adding an endpoint to MCS (PCS) where we can POST the HTML and get all the transformations done would feel more natural. We eventually can even change everything in MCS(PCS) to do POST based API - it will be more efficient then what we're doing now. One less round trip, one less load up of HTML from storage => less latency.

@JoeWalsh Please tell when you need this to be done so that we could plan accordingly.

@ssastry that behavior is fine for references defined elsewhere in the article

@Pchelolo no urgent deadline on our end. This is a nice-to-have for consistency, we can continue to use MW in the meantime

bearND triaged this task as Low priority.EditedJan 11 2019, 10:46 PM

Setting priority to low since the backend work for this seems quite complicated and the workaround of using MW API seems sufficient currently.

Adding Parsoid into the mix would not be my preferred way to go.

Yeah, sorry meant to say talk to Parsoid via RESTBase. I did not men to imply proposing MCS to talk directly to the Parsoid servers.

JoeWalsh added a comment.EditedWed, Jul 24, 5:28 PM

@bearND instead of having clients POST the HTML into the endpoint, could this be a static page with the current mobile-html styles and js that clients can inject a section of parsoid HTML into for display? So the clients would load the static page then call something along the lines of pagelib.c1.preview("<section>...parsoid html here... </section>")

@JoeWalsh I think that could work as long as you can get the Parsoid HTML for the edited section.
I guess clients could POST the wikitext to https://en.wikipedia.org/api/rest_v1/#/Transforms/post_transform_wikitext_to_html or similar for that.

Should the preview show the whole page (but scroll to the edited section) to account for the reference issues @subbu mentioned above?
Either way I think the preview code in the pagelib should at least build the section header for the edited section, too.

Yes, the clients would get the Parsoid HTML for the edited section.

The preview should be a single section for now - we're ok with the issues @ssastry mentioned above.

Thinking about this some more I think it might be easier to just use the /page/mobile-html output the app got for reading purposes and replace the edited <section> with the new HTML for it.

@bearND that'd probably work as well - so it would just be something like pagelib.c1.PageMods.replaceSection(id, section)

could also add pagelib.c1.PageMods.replaceContent(content) if we wanted to preview the entire article

bearND added a comment.EditedWed, Jul 24, 6:01 PM

@JoeWalsh I was thinking of something like pagelib.c1.PageMods.replaceSection(id, sectionHtml). The section ID is non-negative for editable sections. So the section ID we get from Parsoid should be fine for this case.

As long as the apps only edit one section at a time we don't need replaceContent(content). Once that becomes a requirement we could look into that as well.

What we do need in addition to the above is:

  • add a "preview" flag to make sure the edit pencils are hidden via CSS.
  • scroll to the edited section

Both seem relatively easy to me.

JoeWalsh renamed this task from [FEATURE REQUEST] Endpoint that renders user edit previews in the same format as mobile-html to mobile-html: ability to preview an edited section by substituting it into the content.Thu, Jul 25, 6:03 PM
JoeWalsh raised the priority of this task from Low to High.
JoeWalsh updated the task description. (Show Details)
JoeWalsh moved this task from Backlog to Kanban on the Reading-Infrastructure-Team-Backlog board.

Forgot one (not insignificant) thing that we also need:

  • Move some of transformations from PCS to the pagelib code base. Then we could run those from inside the app.

That is needed unless we're fine displaying pure Parsoid output for the edited section.

I guess we could try without first and see how that goes.

JoeWalsh claimed this task.Fri, Jul 26, 4:46 PM
JoeWalsh renamed this task from mobile-html: ability to preview an edited section by substituting it into the content to mobile-html: ability to preview an edited page or section with the same transforms and styles as mobile-html.Fri, Jul 26, 8:42 PM
JoeWalsh updated the task description. (Show Details)

Change 525967 had a related patch set uploaded (by Joewalsh; owner: Joewalsh):
[mediawiki/services/mobileapps@master] Add endpoint for POSTing Parsoid HTML to get a mobile-html preview output

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

Could you please write up the plan on how will this work? Since the POST endpoint will have to be exposed via RESTBase, I need more info into what needs to be done here.

@Pchelolo clients will POST Parsoid HTML to /page/html/to/mobile-html with the same parameters in the path as mobile-html - :title/:revision?/:tid?. The request body will be the parsoid HTML as text, clients need to set the Content-Type header to text/html. The endpoint will return the parsoid html that clients posted with the mobile-html transforms applied.

There proposed endpoint is implemented here: https://gerrit.wikimedia.org/r/525967

Pchelolo added a comment.EditedMon, Jul 29, 7:58 PM

clients will POST Parsoid HTML to /page/html/to/mobile-html

I think on the RESTBase side it would be nicer to expose the endpoint under /page/transform/html/to/mobile-html as it will fit into the current structure of the transform endpoints.

with the same parameters in the path as mobile-html - :title/:revision?/:tid?

What's the reason for doing so? For other transformations (to wikitext) we requite title, revision and tid in order to fetch the matching data-parsoid from storage since Parsoid requires it. How will the parameters be used here?

I think on the RESTBase side it would be nicer to expose the endpoint under /page/transform/html/to/mobile-html as it will fit into the current structure of the transform endpoints.

Works for me

What's the reason for doing so? For other transformations (to wikitext) we requite title, revision and tid in order to fetch the matching data-parsoid from storage since Parsoid requires it. How will the parameters be used here÷

Title is used to grab the article metadata for the header. Revision and tid look to be necessary, I just brought them over from mobile-html in case they were somehow being used in forming the content. I will remove them.

I think on the RESTBase side it would be nicer to expose the endpoint under /page/transform/html/to/mobile-html as it will fit into the current structure of the transform endpoints.

I think it should be /transform/html/to/mobile-html (not under page)

I think on the RESTBase side it would be nicer to expose the endpoint under /page/transform/html/to/mobile-html as it will fit into the current structure of the transform endpoints.

I think it should be /transform/html/to/mobile-html (not under page)

Oh.. indeed :)

Change 525967 merged by jenkins-bot:
[mediawiki/services/mobileapps@master] Add POST endpoint to get mobile-html edit preview

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

As I understand, MCS implementation for this is completed and we now need to add it to RESTBase.

I have a few questions here:

  1. Do you need /transform/html/to/mobile-html exposed publicly, or actually you need transform/wikitext/to/mobile-html - as I understand the idea is for the app to submit edited wikitext, not HTML? Thus, we can combine transform/wikitext/to/html and transform/html/to/mobile-html inside RESTBase and not have the app do an additional round-trip.
  2. Do you need language-variants support of the result?

Do you need /transform/html/to/mobile-html exposed publicly, or actually you need transform/wikitext/to/mobile-html - as I understand the idea is for the app to submit edited wikitext, not HTML? Thus, we can combine transform/wikitext/to/html and transform/html/to/mobile-html inside RESTBase and not have the app do an additional round-trip.

transform/wikitext/to/mobile-html would be better for the clients and eliminate an extra network call, so if that's doable that'd be ideal

Do you need language-variants support of the result?

Yes

Merged. To be deployed soon(TM).

@mobrovac thanks! I'm going to deploy the mobileapps change in the services window today.

Merged. To be deployed soon(TM).

@mobrovac thanks! I'm going to deploy the mobileapps change in the services window today.

Ah, the PCS counterpart is not live in prod yet? Please @MSantos ping us here on the ticket once that's the case.

MSantos added a comment.EditedMon, Aug 12, 3:44 PM

Merged. To be deployed soon(TM).

@mobrovac thanks! I'm going to deploy the mobileapps change in the services window today.

Ah, the PCS counterpart is not live in prod yet? Please @MSantos ping us here on the ticket once that's the case.

It isn't, it is on the queue to be deployed in the next window

@mobrovac it was deployed! My tests were wrong because restbase isn't deployed yet :facepalm: and I was mislead by the task status (to deploy). Anyway, prod is up to date with recent changes and this task is ready to sign off

Mentioned in SAL (#wikimedia-operations) [2019-08-13T15:49:05Z] <ppchelko@deploy1001> Started deploy [restbase/deploy@8fca708]: Expose transform/wikitext/to/mobile-html endpoint T211026

Mentioned in SAL (#wikimedia-operations) [2019-08-13T15:56:40Z] <ppchelko@deploy1001> Finished deploy [restbase/deploy@8fca708]: Expose transform/wikitext/to/mobile-html endpoint T211026 (duration: 07m 35s)

Mentioned in SAL (#wikimedia-operations) [2019-08-13T15:56:58Z] <ppchelko@deploy1001> Started deploy [restbase/deploy@8fca708]: Expose transform/wikitext/to/mobile-html endpoint T211026, take 2

Mentioned in SAL (#wikimedia-operations) [2019-08-13T16:07:10Z] <ppchelko@deploy1001> Finished deploy [restbase/deploy@8fca708]: Expose transform/wikitext/to/mobile-html endpoint T211026, take 2 (duration: 10m 12s)