This edit fixes the gadget and the rendering.

I wonder if https://gerrit.wikimedia.org/r/c/operations/puppet/+/1087230 caused this.

This affects all request URLs with a revision parameter. For example, https://en.wikipedia.org/api/rest_v1/page/html/Hospet returns a valid response but https://en.wikipedia.org/api/rest_v1/page/html/Hospet/1254733652 returns 404 even though that is the latest revision of the page.

Given the pipeline refactoring we did for T363421 and followup patches, it should be relatively straightforward to do T346196#9752972 -- will explore tomorrow.

Looks like the CSS has been designed assuming category and other such metadata markers will be stripped from output which is a bad assumption for Parsoid. Anyway, this one is tricky. Parsoid emits explicit markup for metadata found in wikitext. For top-level pages, this is essential for accurate roundtripping. For templated content, this is essential to ensure that we treat page fragments as true representatives of what a template outputs. Without that, things like selective updates and other interventions will not work because in order to update the page-level metadata, we need to accumulate metadata from all generated fragments by walking the DOM. We can think of alternative representations where metadata is stored out-of-band, but it complicates logic because we have to look at different places and do different things for top-level and non-top-level content, so it is non-ideal.

But, not sure we can easily make that href encoding change since there might be clients consuming Parsoid HTML now which would break if we make this change.

Turns out it is this line in the CulturalHeritageImagesCount gadget that breaks with Parsoid HTML. The category name is here.

Given @AntiCompositeNumber's input and the merge of the patch to handle that, I am going to resolve this task.

Looking at the page source for the legacy parser output, the linked text in the HTML is галерея like with Parsoid. Looks like some JS script is adding the additional (0) or (33) or whatever else is present.

In T378629#10281775, @ssastry wrote:

Actually, I take that back. In the html2wt direction, Parsoid doesn't emit the empty "|-" in the first case as it is. So, deleting it from the DOM before serializing to HTML should be fine.

Actually, I take that back. Parsoid doesn't emit the empty "|-" in the first case as it is. So, deleting should be fine.

Thanks for that useful analysis @Izno! Actually, not sure you need the "|-" before the caption -- so it can simply be deleted. See below Parsoid output (after stripping data-parsoid attributes from the output).

I filed T378733 for followup post readview rollout.

I cam here to close this ticket and see @Tgr's comment. I don't have any intuition about this personally -- so, perhaps @Dreamy_Jazz can weigh in here as the author of the original patch.

We don't necessarily have to block on all the subtasks while deciding on whether to roll out to dagwiki but I am just recording all the issues so we don't lose track of them.

I published the results of analyzing visual diff testing results on wiki and have added at least one blocker task to this ticket based on that. I will add additional subtasks as required.

The visual diff run is done and we have results -- we can rerun results again later once we've fixed any relevant issues.

This gives us a total of 40821 titles to sample from. This is how the titles are spread across those 21 wikis. We could conceivably run diffs on all 40K titles since most pages are likely small.

7696 Wp/mag
5080 Wp/grc
3494 Wp/syl
3181 Wt/tcy
2941 Wy/id
2687 Wp/rki
2314 Wp/iba
2301 Wq/ha
2117 Wt/ary
2095 Wp/isv
1413 Wp/knc
1171 Wp/lua
1059 Wp/tdd
 695 Wp/nup
 594 Wp/tig
 529 Wp/rsk
 479 Wp/ann
 333 Wy/hbs
 268 Wp/nr
 230 Wp/mrt
 144 Wb/bcl

It turns out that wikisource incubator wikis are not hosted on incubator.wikimedia.org which is lucky for us because Parsoid doesn't fully support wikisource yet and so we don't have look into wikisource wikis for now.

Actually, I was not entirely right about deletion -- looks like pages for graduated wikis are removed from incubator eventually. https://incubator.wikimedia.org/wiki/Incubator:Site_creation_log shows that Incubator pages do get deleted (24th April 2024 shows the last deletion for a wiki --although there are wikis from earlier that don't have a deletion log).

Looks like incubator wiki has a large number of wikis and wikis that transition to being their own wiki don't get their pages removed from Incubator. So, we need to factor all of this while creating a suitable random page sample from Incubator. Ideally, we would have a test sample between 10K - 30K pages across all wikis. So, for this to be a meaningful sample, we may have to narrow our focus to a smaller subset of wikis. We could focus our attention on the most active wikis. We can also use https://incubator.wikimedia.org/wiki/Incubator:Site_creation_log to guide our sampling decisions by excluding them from our sampling since Parsoid will not be enabled by default on those wikis.

I ran rt-testing twice last week and there are no regressions. Logstash shows no (new) errors from the test runs.

Summary based on a slack discussion between @Reedy, @cscott , and me.

I looked at the discussion here, the subtasks, and the ContactPage code, consulted some folks, and here is my understanding.

• @Dreamy_Jazz replaced use of isRegistered with isNamed as part of T344722 to ensure real names are only used for non-temp registered users (as is the intention).
• Previously, anon users of the contact form would have their IP sent over unconditionally. But, registered users would get a checkbox to choose if their IP is sent. With temp users, the expectation is that we are going to also present them with a checkbox to give them the option to hide their IP. If that is true, then there is no other change necessary to the code and this task is resolved.
• But, if the product expectation is that temp users using ContactPage don't get to hide their IPs, the code need to be tweaked slightly -- easy to make the change.

Removing $cfg['null_casts_as_any_type'] = true; is still a todo. It is still very spammy. Unassigning myself for now -- not planning to work on this anytime soon.

https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/1075350 (after a minor tweak) will take care of this.

In T375539#10172372, @Izno wrote:

Scribunto's errors have a click-over effect that lets you see how the errors showed up (a stacktrace), which I would guess is why it has an ID. Can the same be done with a class?

I think this is a case of the template and ''' not separated by anything which triggers some edge case.