Page MenuHomePhabricator

Editing section opens wikitext for previous section
Closed, ResolvedPublic

Description

Reported from eswiki user Bikendi1991 in the #wikimedia-ayuda irc channel, channeled to #wikimedia-mobile by @-jem-

Device: Android 5, Nexus 4. Also reproduced in a tablet.

Steps

  1. Visit https://es.m.wikipedia.org/wiki/Copa_del_Rey_de_Baloncesto_2017 in the android application
  2. Try to edit section Sorteo or any of them below that one

Expected

  1. Editor loads with wikitext for the Sorteo section

Actual

  1. Editor loads with wikitext for the Cabezas de serie part

Seems like the usage of headings inside the columns template is confusing the app's section detection. This doesn't happen in mobile web. I haven't verified if it happens on ios.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Thanks, @Jhernandez. To give proper credits, the bug was first reported by eswiki user Bikendi1991 in the #wikimedia-ayuda IRC channel; then I reproduced it and searched for help :)

The table of contents links are mismatched on this article as well

Dbrant subscribed.

Note: it works correctly when using MW, and incorrectly with RB. Adding tag :)

Change 385422 had a related patch set uploaded (by BearND; owner: BearND):
[mediawiki/services/mobileapps@master] Get section number from Parsoid

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

The example page has been deleted. But I found other examples when going through top 1000 pages in frwiki:

  • fr:Mary_Pierce/138725507, section "Parcours en Coupe de la Fédération" or later
  • fr:Eminem/140492693, section 37 "Tournées" or later
  • fr:Kristina_Mladenovic/140586464, section 21 "Parcours aux Jeux olympiques" or later

I believe this ^ MCS patch should fix it but have a hard time verifying as of right now since I keep running into timeouts in the Android app when going through my local Parsoid instance.

QA: Note that verification of this depends on both this MCS patch being deployed and on Parsoid deploying T114072.

This patch (in conjunction with the proposed Parsoid change) seems to work well, but I want to make sure I understand the acceptance criteria. The problem here, in general terms, is "disagreement between our read HTML section parser (currently MCS) and our edit HTML section parser (currently the MW PHP parser) about what constitutes a new section." Parsoid (as updated in Subbu's series) seems to agree with the PHP parser in most cases; at least I haven't run across any exceptions yet. But is it actually the goal of the new Parsoid sectioning code (our proposed read HTML section parser) to agree with the PHP parser in all cases? That isn't clear to me from reading the discussion on T175305.

It seems like the sole acceptance criterion here for your (@bearND's) patch is that it accurately passes through sections as marked up by Parsoid. Hence, it would be mergeable even if Parsoid's sectioning differed substantially from the PHP parser's and this bug was still outstanding. (Happily, though, it seems they mostly agree, and merging should fix most if not all instances of this bug once Parsoid sectioning is merged and deployed.)

Does this sound about right or am I off base?

Ah, I see from https://gerrit.wikimedia.org/r/#/c/364933/27//COMMIT_MSG that it is indeed intended for Parsoid sectioning to match the PHP parser's. If all else fails, read the commit message. :)

Change 385422 merged by jenkins-bot:
[mediawiki/services/mobileapps@master] Get section number from Parsoid

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

The MCS change is deployed as of a few minutes ago. That doesn't mean it's active, though. It will be active once Parsoid adds the section tags (in version 1.6).