In T222988#5178670, @ssastry wrote:

Besides the 3 there, there is also Includes and comments at SOL that needs investigation.

In T222988#5178062, @ssastry wrote:

In T222988#5174567, @ssastry wrote:

With these fixes, we are down to 3 failures (which fail for known reasons)

T107652: <ref>s in templates that also generate table cell attributes should be rendered properly <-- TOKENIZER related which cscott is working on
<pre> inside a link                                                                               <-- KNOWN failure because of bad Parsoid DOM which doesn't roundtrip across serialization and parse
{{!}} is a magic word                                                                             <-- KNOWN failure possibly because of some unported code

So, I think we are good for now here.

I lied it seems. I broke more things by the refactoring in later patches while fixing those other failing tests. While testing, I ran just those failing tests instead of a full test run. Anyway, will take a look today.

In T222988#5174567, @ssastry wrote:

With these fixes, we are down to 3 failures (which fail for known reasons)

T107652: <ref>s in templates that also generate table cell attributes should be rendered properly <-- TOKENIZER related which cscott is working on
<pre> inside a link                                                                               <-- KNOWN failure because of bad Parsoid DOM which doesn't roundtrip across serialization and parse
{{!}} is a magic word                                                                             <-- KNOWN failure possibly because of some unported code

So, I think we are good for now here.

With these fixes, we are down to 3 failures (which fail for known reasons)

T107652: <ref>s in templates that also generate table cell attributes should be rendered properly <-- TOKENIZER related which cscott is working on
<pre> inside a link                                                                               <-- KNOWN failure because of bad Parsoid DOM which doesn't roundtrip across serialization and parse
{{!}} is a magic word                                                                             <-- KNOWN failure possibly because of some unported code

So, I think we are good for now here.

In T222988#5174099, @gerritbot wrote:

Change 509592 had a related patch set uploaded (by Subramanya Sastry; owner: Subramanya Sastry):
[mediawiki/services/parsoid@master] Hybrid testing: Pass env.fragmentMap for pipeline stages

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

In T217867#5156705, @jlinehan wrote:

In T217867#5156656, @ssastry wrote:

Nice work! Quick initial comment now. As far as Parsoid is concerned, assuming feature parity wrt functionality, performance will be the biggest determining factor here since Parsoid's functionality is heavily DOM based.

I'm thinking an initial sanity check could be performance vs. PHP DOM on generic tasks, yes? What could be a good representative task / set of tasks? Build large document? Read, parse (with RemexHtml), modify, write? At least to get a handle on whether it's off by an order of magnitude or something and just isn't going to work.

In T222992#5173749, @cscott wrote:

...though it seems impossible, I've confirmed that 3058052c756ac7b69ead21b4b237a1cd6714de8a in Remex causes these failures. (If only I knew why!)

I have marked all these tasks low for now. We'll revisit them post-port.

Okay, I'll chat with Scott and Tim about this and figure out who can tackle this.

In T219933#5167290, @Dzahn wrote:

In T219933#5167021, @jbond wrote:

did this issue get fixed? i just checked scandium and parsoid-vd has been running for 3 weeks

Thanks! Based on this and also on:

This is not critical. I asked @Dzahn to turn off these alerts for now. Meanwhile, I started a test run to confirm that the service
doesn't terminate when there is activity against the service.

I would say we can close this. @ssastry if you disagree we can of course reopen it right away.

@Dan1wang thanks very much for all the phab tasks exploring the PEG tokenizer with suggestions for possible simplifications / improvements of the rules. We appreciate the attention and effort you've been putting in here.

In T221907#5165295, @sbassett wrote:

@ssastry - looking at https://gerrit.wikimedia.org/r/q/project:mediawiki%252Fservices%252Fparsoid - are most of the patch sets here related to the new PHP-Parser development? Is there a specific start date or branch/topic we should be paying attention to right now for the new development? Or does master for mediawiki/services/parsoid fulfill that role right now? Trying to understand if it's worth the Security-Team's time to have a look at recent patch sets or if we should wait for a more stable release branch, etc. Thanks.

Sorry, we are heads down in the porting world and we cannot investigate this deeply at this point. But, at first glance, yes, it seems like some bug in Parsoid. I ran a --trace peg and it took a while before that output stopped. But after that, I didn't see any activity. So, probably it got stuck elsewhere in the pipeline. We have --trace options for almost every stage in the pipeline. So, we'll have to play with them or if that doesn't work hook up a debugged to see what is going on. But, this is not something we can take a look at in the immediate term.

In T222639#5163458, @mobrovac wrote:

@ssastry @Arlolra so the TL;DR is: if we have the exact render data-parsoid, send it, otherwise just send the modified HTML. Is that assessment correct?

In T222607#5161193, @marcella wrote:

@matmarex thinks this is likely to be a Parsoid issue. @ssastry please let us know if your team disagrees.

In T222639#5160906, @Pchelolo wrote:

(b) so, should we send no data-parsoid entirely, or send 'whatever we have even if not matching'?

@Pchelolo: this doesn't just affect VE but all Parsoid clients that might fetch html for editing and might then post edited html for save. So, couple of comments (a) this should actually be documented in the API docs (swagger?) alongwith the implications of what happens if a client doesn't comply ... which brings me to the next point (b) if data-parsoid is missing, we should make sure RESTBase sends data that Parsoid serializes correctly ... the output should still be correct but it may be normalized (and hence cause a dirty diff).

In T217867#5156100, @jlinehan wrote:

As I previously mentioned to @cscott, who provided useful early guidance, I spent my 10% time in April working on a port of Domino.js to native PHP. Most of the core DOM functionality that was covered by Domino.js is complete, so I thought I ought to let folks know this has been going on. I didn't want to "lick the cookie," in case I ended up not getting very far.
...
Anyone who wishes to be involved is welcome. I'll also continue working on it, but now that actual code exists, it might be good to talk specifics so that development goes in a direction beneficial to the Parsing team or whoever else could use it (T218183).

StructuredDiscussions folks: note that since you don't talk with RESTBase, you will need to update your code and config to use the new Parsoid-PHP endpoints. I'll create a phab task for you. But this is FYI. You might also be interested in following T221738: Parsoid REST API routes in MediaWiki more specifically.

We are currently targeting a Parsoid-PHP deploy in Q1 2019/20. So, ideally, this task would be resolved by mid-July.

Once porting is done, we are unlikely to use this test mode much. And given hybrid testing is working fairly well, this testing mode doesn't have as much utility as it did in the early days of porting. We can revisit this if this testing mode has utility post-porting.

With https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/505681, we have initial CI testing in place now. This can be extended as more code is ported.

Because of this bug, a number of cells on this page don't get a background colour.

In T221988#5149289, @Anomie wrote:

probably review the phab task for dependency injection. the constructor or entry point should probably take an abstract object for Env, I think that's the point at which all our "dependencies are injected"

I don't think we want to inject an Env. The plan I had was that we'd inject a SiteConfig into the constructor, and pass a PageConfig and DataAccess into the entry point methods as it's possible for MediaWiki to want to parse multiple different blobs of wikitext within the course of one request (and ideally without having to reinitialize Parsoid each time). The entry point methods would combine the three to create the Env, which would be completely internal to Parsoid-PHP.

In T222064#5143647, @Marc-Andre_Assbrock wrote:

@ssastry What do you think about mentioning it on https://www.mediawiki.org/wiki/Parsoid/Setup ?

In T221907#5143839, @sbassett wrote:

@ssastry - thanks for submitting this. I was just curious if there's a more concrete deployment date in mind than "Q1 2019/20". That would help us a bit more with our scheduling.

We are currently focused on the PHP port and won't be able to attend to this for a while.

Yes, the correct solution is to address T94603.

In T221907#5140695, @cscott wrote:

I recall that we did have the issue with Parsoid being exposed to sensitive content, specifically usernames and deleted revisions. The details are foggy, but I vaguely recall that I had to remove some information from the <head> of the Parsoid document because it could expose information about deleted revisions.

I would like to explicitly separate two projects: (a) deployment of Parsoid/PHP as a replacement for Parsoid/JS (b) making Parsoid/PHP the default wikitext engine on Wikimedia cluster.

In T221876#5138264, @cscott wrote:

This seems pretty low priority; W3C is following the WHATWG spec so there's no real difference. WHATWG tends to lead, and then their decisions are ratified by W3C sometime later.