Page MenuHomePhabricator

Implement a sane code-review process for MediaWiki JS/CSS pages on Wikimedia sites
Open, LowestPublic

Tokens
"Love" token, awarded by TerraCodes."The World Burns" token, awarded by Liuxinyu970226."Like" token, awarded by MGChecker."Love" token, awarded by Huji."Yellow Medal" token, awarded by Ltrlg."Orange Medal" token, awarded by Krinkle."The World Burns" token, awarded by yuvipanda.
Assigned To
None
Authored By
Legoktm, Aug 12 2014

Description

MediaWiki: CSS/JS pages on any non-large wiki are usually a mess. Occasionally, we'll discover that wikis have been loading external resources for months and no one noticed. In addition, the local sysops maintaining those pages usually don't know JavaScript and are copy-pasting what someone else told them to do. Even when that's not the case, mistakes can result in code with obvious syntax errors being sent to readers for long periods of time.

Various proposals have floated around over the years. The wikitech-l threads have some good discussion on some of the reasons why this is difficult for smaller wikis.

See Also:

Details

Reference
bz69445

Related Objects

StatusAssignedTask
Declined Reguyla
OpenNone
ResolvedNemo_bis
OpenNone
DuplicateNone
OpenNone
OpenNone
OpenNone
OpenNone
DuplicateNone
StalledNone
OpenNone
OpenNone
ResolvedTTO
ResolvedNone
OpenNone
ResolvedNone
ResolvedNone
StalledNone
ResolvedLegoktm
OpenNone
DuplicateNone
ResolvedSamwilson
ResolvedNone
OpenNone
OpenNone
OpenRical
DeclinedNone
OpenNone
OpenNone
OpenNone
OpenNone
ResolvedNone
OpenNone
OpenNone
InvalidJackmcbarn
Openthiemowmde
DeclinedRical
OpenRical
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenTK-999

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

I've been thinking about this task a bit more with T151408: Separate bot right for normal pages and interface (MediaWiki:) pages recently filed. I wonder whether we could implement some kind of social +2 process for on-wiki JavaScript and CSS as a medium-term solution. If we needed to, we could also investigate a technical +2 process, but that's probably overkill for now.

What would a social +2 process look like? We'd try to convince the various wikis to adopt a process of proposing edits on the talk page and having more than one person review the code for sanity before editing the live JavaScript or CSS pages. This type of approach has both the same benefits and detriments as Gerrit's +2 process, of course. Gerrit implements this at a technical level and wikis could as well if needed, but a social process would be more organic and perhaps a better starting point.

A lot of the discussion surrounding security issues related to on-wiki JavaScript editing boils down to this task, I think. A true increase in security would likely only come from some kind of sane and workable code review situation. The other issues (such as bot flags) are peripheral and marginal in comparison.

demon removed a subscriber: demon.Nov 27 2016, 10:16 PM
Tgr added a comment.Feb 3 2017, 9:28 AM

A couple possibilities:

  • write a bot that syncs page content to files on Github (or some other DVCS site to be bikeshedded later), push out the whole code review system there
  • write a proper extension that does that and can load alternative branches / pull requests on demand, a bit like TemplateSandbox
  • repurpose CodeReview to work with page revisions instead of an SVN repo, and pair it with something like FlagRev.

The second option is probably the sane one.

This proposal is selected for the developer wishlist voting round and will be added to a MediaWiki page very soon. To the subscribers, or proposer of this task: please help modify the task description: add a brief summary (10-12 lines) summarizing the problem that this proposal raises, topics discussed in the comments, and a proposed solution (if there is any yet). Remember to add a header with a title "Description," to your content. Please do so before February 5th, 12:00 pm UTC.

Tgr updated the task description. (Show Details)Feb 5 2017, 7:47 AM
He7d3r renamed this task from Implement a sane code-review process for MediaWiki JS/CSS pages on Wikimedia sites to Implement a sane code-review process (use git?) for MediaWiki JS/CSS pages on Wikimedia sites.Feb 5 2017, 12:51 PM
Izno renamed this task from Implement a sane code-review process (use git?) for MediaWiki JS/CSS pages on Wikimedia sites to Implement a sane code-review process for MediaWiki JS/CSS pages on Wikimedia sites.Feb 5 2017, 1:49 PM
Tgr added a comment.Feb 15 2017, 9:38 AM

This was the second (slash third, in a tie) most popular item in the Developer Wishlist results. Any thoughts about the next step? I guess first we should determine if this needs to be done inside MediaWiki or the community would accept something based on gerrit/github/whatever (which would be much less work and would provide much better tooling integrartion).

Krd removed a subscriber: Krd.Feb 15 2017, 9:42 AM
In T71445#3028613, @Tgr wrote:

I guess first we should determine if this needs to be done inside MediaWiki

Yes please.

or the community would accept something based on gerrit/github/whatever (which would be much less work and would provide much better tooling integrartion).

Gerrit is awful [no citations needed] and GitHub is proprietary, and plenty of people -- developers and users alike -- care about the solution being as FOSS as possible.

Anyway, I don't think doing this as a MediaWiki extension is impossible; there are probably a few tricky questions to solve and whatnot, but considering that Wikia managed to implement a similar system, it should be doable. I'd be interested in helping out where possible to get this thing built.

If this could be done as an extension that talks to any git repository (as configured by site admins), then it would work with whatever Git code review system people want (i.e. Wikimedia sites could use Gerrit or Differential), and third-party wikis could do it with Github.

An extension could easily update a given set of gadget's pages based on the content of files in a public Git repository somewhere. It'd be really cool if changes made locally could also be pushed out to that repository as review-requests — although perhaps it's more likely that the set up would be that some wikis are configured to only pull from the repo (because that's the point of requiring code review after all), and it'd be possible to configure development wikis to send new code to the repo (and to receive updates when the user asks for them).

Is there a possibility that such an extension could be a sort of global gadget system? The code is still duplicated, but it's controlled centrally. Would work for templates and modules too.

Everybody is welcome to contribute ideas, create tasks, start polls, etc. but you need an engineer to do the engineering. When I say "engineers" I'm not referring to Wikimedia Foundation staff. Instead, it should fully embrace open source.

I agree with most of what you write in this comment. I'll note that there's a view that engineer should be treated similar to a term like doctor, and should require at least a degree in engineering. Developers, coders, or programmers might be more apt.

I also don't really love the notion that engineers need to do the engineering (even when broadening the terms to be almost meaningless). Wikipedia is not exclusively written by professional encyclopedists, Wiktionary is not exclusively written by professional lexicographers, Wikinews is not exclusively written by professional reporters, etc.

In T71445#3028613, @Tgr wrote:

This was the second (slash third, in a tie) most popular item in the Developer Wishlist results. Any thoughts about the next step?

I would recommend re-reading this task. It's quite long, but contains a lot of good and useful thoughts.

Speaking of engineers and engineering, it strikes me as odd and interesting that the Gerrit code review process isn't really well-liked (cf. T114419: Event on "Make code review not suck", etc.), and yet we're expecting the same people to build a "sane code-review process" that integrates perhaps Git, perhaps wiki pages, and definitely an even larger and more diverse group of Wikimedians?

@MZMcBride: the difference between on-wiki JS and CSS and pretty much any other wiki page (although templates and modules sometimes also fall into this category) is that edits to them can break the whole site for other users. So it's not about trying to limit editing of these to "professionals", but just to make it less likely to introduce errors.

Which is why these pages are often protected and left to admins to edit. But even people who know what they're doing (which is all I would take "engineers" in the above sense to mean) can make mistakes and would benefit from a process of being able to propose changes and have those changes be easily reviewed by other people before they go live and suddenly make everyone's search autocomplete stop working. :-)

I'd also say that no one seems to be suggesting Gerrit as a solution. I'm not super familiar with Differential, but I guess it's better (or does it just have more whitespace and better-looking buttons?) On the other side of the fence, there's Github and seemingly a lot of people find that relatively easy to use. So surely we can have figure out something that works.

The tricky part of it I think is whether or not it's going to be possible to set up a development environment (or e.g. use an existing test wiki) in which to work on scripts etc. and then easily move them from there to code review and then to the live wiki.

scfc added a comment.Feb 20 2017, 11:51 AM

[…]
The tricky part of it I think is whether or not it's going to be possible to set up a development environment (or e.g. use an existing test wiki) in which to work on scripts etc. and then easily move them from there to code review and then to the live wiki.

That's why I'd prefer a more universal approach: If you think of a wiki as a whole as a Git (or similar) repository, you could test in a particular "branch" (even if for example a change in a template system requires edits to multiple templates pages) and, after testing, merge it to "master". Proposed changes to protected pages could be submitted as branches and, if approved, merged. Changes to (subelements of) the main page or other portals could be mocked up without having to fiddle with all the dependent links. Flagged revisions would be a special case where there is a social contract to merge changes to a particular branch regularly. Users could save their incomplete rewrites of a page in a branch without having to copy & paste it to their computer. Etc.

That wouldn't make the solution to this task easier, but if the solution to this task already is complex, it would be nice if it solves other use cases as well.

Note that, for templates and modules, a branch-like system is already used manually on English Wikipedia (and probably other wikis) via template/module sandbox pages, though there's no hard requirement for their use to propose changes.

Just an idea: what about committing to have an initial plan agreed by the end of Wikimania -- latest? Just an arbitrary date, but a firm one. A driver would be required to stir the discussion and draft plan, though.

Trizek-WMF updated the task description. (Show Details)May 21 2017, 2:26 PM
TK-999 added a subscriber: TK-999.May 22 2017, 1:11 AM

Based on my work around T165981 during the 2017 Hackathon - which essentially involved creating a prototype implementation of Wikia's ContentReview extension compatible with core MediaWiki - I would like to note the following:

Creating an extension that just requires site JS to pass certain requirements (manual review, automated tests) before it is served to other users is relatively easy to develop. As discussed above and elsewhere, such a system has been in operation at Wikia since 2015 - its main benefits are that it has a clear workflow, and is tightly integrated into MediaWiki.

I'm on the opinion that if extra code review features are required from this product, such as branching or line comments, implementing them within MediaWiki would be a bad idea. It would have to reconcile MediaWiki's concept of singular multi-revision pages with the paradigm of a branching VCS - an extension implementing this could easily become a highly complex behemoth that is a nightmare to develop, maintain and sustain.

If the decision is to extract the code review functionality from MediaWiki, it is possible to leverage the power of existing VCS and code review tools, such as Gerrit. In this scenario, the setup can be further simplified by completely eliminating JS pages handling from inside MediaWiki. The versatility of ResourceLoader makes it possible to define configurable custom modules that pull their source content from an external data source instead of MediaWiki pages.

Huji awarded a token.Jun 5 2017, 5:51 AM
Huji added a subscriber: Huji.

See also T171563: Only allow MediaWiki, Gadget, and User namespace pages to be treated as JS or CSS (no project namespace, etc.), which I just created for the rest of the problem (pages outside MediaWiki: namespace, still a very large set with some potential vulnerabilities).

Samtar added a subscriber: Samtar.Jul 25 2017, 6:55 AM

In relation to T158149: Find an owner for top 10 Developer Wishlist 2017 proposals, I dare to ask: what is the current status? :)

Would it be more secure to possibly add in a few more custom user permissions such as splitting editinterface into editJS and editCSS with also adding in restrictions for template namespace and editLua for Lua? I know that edituserjs&css was split into edituserjs and editusercss. This way the mediawiki namespace would contain the interface messages; all javascript is restricted to .js and users with editJS, all css is restricted to .css and users with editCSS; all lua modules restricted by editLua; possibly restrict templates to editTemplate. Just a suggestion...

From what I remember that was already done in Gadgets-2.0.

This comment was removed by SpookyGhost8.

There also is Extension:CodeReview but status is historical reference, perhaps reopening that for development would be a potential solution?

There also is Extension:CodeReview but status is historical reference, perhaps reopening that for development would be a potential solution?

CodeReview is perfectly stable and usable and (mostly) feature-complete. The {{Historical}} template was added to that page by someone who doesn't appear to be familiar with the extension. There is absolutely nothing preventing you from installing and using CodeReview -- or if there is, that's a bug and I'd love to hear about it so that we can get it fixed! Just because something isn't used on Wikimedia's wikis anymore doesn't mean it's flat-out obsolete and/or unusable.

All that being said, CodeReview is mostly a web UI for a Subversion (SVN) repository. The workflow is essentially different when it comes to a SVN repository as opposed to editing on-wiki .css and .js pages: changes to the latter are live immediately, whereas code changes committed to a SVN repo need to be manually deployed by someone.
It is my firm belief that creating a largely new system for "code reviewing" on-wiki .css and .js pages would be the easiest and cleanest solution instead of trying to repurpose CodeReview. In any case, please feel free to add me as a reviewer to CodeReview patches and I'll be happy to review 'em (unless someone else beats me to it, that is).

Tgr added a comment.Sep 18 2017, 4:34 AM

CodeReview + FlaggedRevs is a possible way to handle this task, although not a good one. SVN-style "linear" history sucks for code review (which is why pretty much every single projetc abandoned it when DVCS-s became mainstream), and implementing non-linear history in MediaWiki does not sound like fun, so the most promising approach is to make MediaWiki sync with the master branch of some remote repo, after which there is no point in rebuilding the code review interface in MediaWiki, since there are plenty decent ones out there.

I think relying on Git is a good idea, but I think you underestimate that by moving contributing to these files to a git, you remove it from direct access from the wiki. I think it will feel like we're taking away control form the community, even if this isn't the intention, as we in some way remove the direct editing possiblity from the wiki and make it more difficult to contribute there for normal user.

We have to find a compromise to minimize this problem, because this wouldn't ever be accepted otherwise. (Many users still remember the js wheel-war between WMF and community at German Wikipedia to well, even if it's been a long time ago and many things have changed.)

another possibility outside of Gadget 2 is a test wiki where any JS, CSS or Lua is tested and then moved over to the main WMF wikis after having a +2 code reviewer approve merging it over. The MediaWiki namespace is mostly interface messages, restricting JS & CSS editing to new permissions might make this safer as well; editusercssJS was split into separate permissions though seems to be limited use cases (someone breaking personal js/css).

Tgr moved this task from Backlog to Next on the User-Tgr board.Oct 31 2017, 12:01 AM
Tgr removed a project: User-Tgr.
In T71445#3613657, @Tgr wrote:

the most promising approach is to make MediaWiki sync with the master branch of some remote repo, after which there is no point in rebuilding the code review interface in MediaWiki, since there are plenty decent ones out there.

Now filed as T187749: Make it possible to use code from an external repository for editor-controlled Javascript/CSS.

SpookyGhost8 added a comment.EditedOct 3 2018, 1:51 AM

I know that a while ago it was mentioned that Extensions Flagged Revs + Code Review could be a potential solution. I tested yesterday on a small wikifarm and Flagged Revs does not work on the MediaWiki namespace at all, pretty sure I configured correctly. I did not grant myself the autoreview permission and did not use any of the default user groups. Any valid or invalid javascript added to MediaWiki:common.js went live immediately. Trying to hardcode "NS_MEDIAWIKI" into extension array resulted in an internal error at Special:UnreviewedPages. Extension:Approved Revs explicitly says that it doesn't work on MediaWiki namespace.

Edit: "addportlet" error was my own script, not Flagged Revs

I know that a while ago it was mentioned that Extensions Flagged Revs + Code Review could be a potential solution.

For the records, CodeReview is unmaintained so you'd have to solve T205482 first.

thanks for letting me know about T205482. Given that Flagged Revs gave me an internal error (VM1844:1 Uncaught TypeError: Cannot read property 'addPortletLink' of undefined) for trying to add in MediaWiki namespace, I do not think that placing effort and time into solving T205482 is worth it.

probably better off creating a new extension either from scratch or reusing elements from Extension Flagged Revs and Moderation. Simple Extension that has Approve/Reject buttons with a special page listing unreviewed .js, .css, Gadgets and preventing unreviewed from going live would be enough for basic usage. Also would allow any wiki to use it, not just WMF.

Last I checked, this task stalled out on deciding a solution.