User:Vahurzpu/Fixing T313262

I recently made my first MediaWiki patch to fix T313262. I'm recording my process in detail in hopes that it might provide hints to other first-time contributors. While I do leave out a few dead-ends, I'm trying to give a comprehensive overview of what I did.

From the app to the API
I picked this task because I had a suspicion about where the issue might be coming from. The appearance of "User:Badtitle/dummy title for API calls set in api.php" made it clear that somewhere, a title wasn't getting set that should have been. I started with the app code itself. Searching for "edit notice" in the Android app code turned up EditNoticesDialog.kt. This doesn't have any code that seems to fetch the edit notices, but looking for where that class was referenced led me to EditSectionActivity.kt. Looking at the invocation, it's right near a method called. This was convenient, as I can debug VisualEditor in my browser more easily than elsewhere.

The bug was caused by trying to edit a userscript page, so I first tried manually loading one of those with. However, that didn't work. Therefore, I found an arbitrary page with an editnotice and opened it in VisualEditor with developer tools open, then looked for an API request that contained an editnotice response. After changing the title in the URL, I found that I could reproduce the bug: loading https://en.wikipedia.org/w/api.php?action=visualeditor&paction=metadata&page=User:Vahurzpu/common.js&formatversion=2 (while not logged in as myself) had "User:Badtitle/dummy title for API calls set in api.php" in it. Therefore, I had a somewhat more straightforward way to reproduce the bug.

From the API to the source code
I still didn't know where the error was originating, so I decided to start poking around for leads. I knew that the output was the result of some sort of internal message, so I started looking for which one. I saw that the message linked en:Wikipedia:User script, so I used WhatLinksHere to see what pages in the MediaWiki namespace linked there. That led me to MediaWiki:Customjsprotected. I then searched for occurrences of "customjsprotected" in Wikimedia-deployed code that wasn't the i18n files. This only gave me two hits, and PermissionManager.php was clearly the right one. The reference was in. The function is private, so it must be used by something else:, which in turn is used by. Looking at the documentation comments for the function, this does look like the sort of thing the VisualEditor API would be using. I then searched the VisualEditor code, and there was one clear reference (for reference: the revision I was looking at). I'm pretty sure I also went the other direction, using Codesearch to look for, the key I saw in the JSON. Both of them brought me to the same block of code.

Reading through the block, I was pretty sure I had found an important spot. I could tell from the context that it was rendering wikitext into HTML, and it didn't seem like there was any reference to the title. That explained where the Badtitle was coming from: since it didn't have an explicit title, it was falling back on.

My original plan wasn't to actually fix the bug, but just to leave a comment explaining with some detail where the problem was, in hopes that the bug would get fixed faster. However, I figured that leaving a VisualEditor fix in an Android app bug wouldn't be particularly helpful, as the app developers wouldn't necessarily be familiar with the internals of VisualEditor, and I wasn't entirely sure where in that block of code the error was arising. Therefore, I decided to bite the bullet and set up a local development environment to debug it.

Setting up MediaWiki
I looked through the Wikimedia Developer Portal, which, after a bit of clicking, led me to How to become a MediaWiki hacker, and from there to MediaWiki-Docker. As it says to do, I cloned the git repository and went through the instructions in DEVELOPERS.md — mostly. I have podman rather than Docker on my laptop (running Fedora 36), but since it's advertised as an almost drop-in replacement, I figured this wouldn't be an issue. Following the instructions in this blog post, I set up docker-compose, then continued, skipping the Xdebug step for the time being. The containers started up, but when I tried to run, I ran into a permission error. Using  to debug, I found that I couldn't access , which was supposed to contain all the code.

Fixing the volume permissions
I'm not going to give a 100% complete explanation here, as it took a long time and was mostly the result of deviating from the instructions, rather than following them and still running into issues. I first thought that the issue was with filesystem permissions. However, I tried using  and   to run things as root, and still couldn't read it. Eventually, I figured out that filesystem permissions weren't the issue. I came across a Red Hat blog post suggesting adding "Z" to the end of the volume mount specifications and running a "podman unshare" command. This didn't work, but did succeed in screwing up my filesystem permissions for the whole directory. Eventually convinced that the problem must lie with SELinux, I tried setting my system's SELinux enforcement state to permissive, and found that it immediately started working, narrowing down the causes. I didn't want to leave SELinux off, so I turned it back on and investigated more. I tried to look through the logs, but found nothing interesting. I came across a blog post that cleared it up a bit. I was able to verify with  that the descriptors were what they should have been, and it also hinted at a distinction between lowercase z and uppercase Z. I had already attempted the former, and so tried the latter. It worked. I then was able to install the dependencies, then MediaWiki itself.

Installing the extensions
After that, I could open up the web server. From there, installing Vector and EventLogging was straightforward, just following the DEVELOPERS.md instructions. I also followed MediaWiki-Docker/Extension/VisualEditor to set up VisualEditor; there were no issues there. I then created another user account and set up a system message to reproduce the bug.

Getting XDebug to work with VSCode
This is another case of a very frustrating debugging process, so I'm not going to go through everything I did in the interim. It took me a considerable amount of time to find MediaWiki-Docker/Configuration recipes/Xdebug and MediaWiki-Docker/Configuration recipes/Xdebug config for VS Code; neither of these were linked from DEVELOPERS.md, but the process spelled out in DEVELOPERS.md didn't work, whereas this did.

Finding the bug
When I got XDebug to connect, I set a few breakpoints. I then checked to see which one had an incorrect value. This confirmed my suspicion that the output of  was reasonable, but that the message introduced the error.

Fixing the bug
From there, I looked at. There wasn't a whole lot in RawMessage.php; it inherits most of its functionality from Message.php Looking throuth that file, I found that it had a  method. Convenient! It also returns itself, so I could just chain it in. Therefore, I inserted a call and verified with my local version that Badtitle was gone.

Uploading the patch
With my patch written, I went to Gerrit/Tutorial. Because I've reported bugs before and have a Toolforge tool, I already had a developer account with SSH keys set up. I had git, and use it for plenty of non-Wikimedia stuff, so I omitted the  when configuring user.name, so that my non-Wikimedia patches retain my actual name. I also found out that there's no automatic uppercasing like there is with SUL usernames. The SSH command worked (again, with a lowercase username), so I moved on. I realized that I had cloned the VisualEditor repository anonymously. While I'm sure there was a better way to do this, I just kept track of the line I needed to change, deleted the old copy of the code, and re-cloned the repository with SSH

I installed and set up git-review, then set it up. Almost all of it worked, but the  command at the end failed. I looked it up and saw a suggestion to add "-O". I tried running that command, which then succeeded. I re-ran the git-review command, and there were no errors.T300184

Setting up the tickets
I hadn't escaped the ticket scope issue: I was still suggesting a VisualEditor fix for an Android app bug. I therefore decided to open a new ticket as a subtask of the Android app bug, have that title be really specific, and tag it with VisualEditor tags. This ticket was T313372. When I was looking around for similar tasks to figure out what other tags to add, I found T300184. This was so similar I took a while to look at the blame and make sure that this wasn't a bug that got accidentally un-fixed at some point. However, I eventually determined that it merely fixed most of the places where this bug could come up, but not all of them. The patch still made sense.

The final push
Then I set up a new branch, re-added my  call, and made my commit. I realized that I needed a bug IDThen I rebased (not that I really needed to, given that it wasn't likely anyone was pushing a change in the very brief period I needed to apply the patch). Then I sent it off for review, and checked that gerritbot commented on the Phabricator task. I also repeated several of the steps again the next morning after I realized I goofed and didn't check the PHP coding conventions, and so didn't include enough space. I added that as another patch set before a reviewer saw it. However, a few hours after my fix, a reviewer looked at it, and the patch was merged.