Page MenuHomePhabricator

Add "test patch" option via address line
Closed, DeclinedPublic

Description

So, often we are informed that there is a patch to review.
in most cases, it will be a php patch.
however, more and more of MW relies on more and more sophisticated JS code, so more and more patches are JS+css.

currently, phabricator users can review the code, but there is no simple way for them to test it, which limits gerrit value to only programmers.

i suggest to teach mediawiki code one more address line parameter: just like we have

&debug=true

which helps debugging JS code by changing the value passed to "src" values in script and style tags, i suggest we teach mediawiki to respect a new

&patch-test=012345

parameter, that will look for patch #012345, and if this patch contains any js or css, it will load the appropriate modified files instead of the ones in the distribution.

(if no such patch, or the patch has no JS or CSS, this parameter can induce a "patch such and such not found" or "patch such and such has no js or css" comment in the html source).

this will allow, in many cases, to ask the original reporter of the bug, and anyone who "confirmed" it with different OSs and browsers, to test the patch, and hopefully comment on the patch with "tested - passed" or "tested, i am still seeing issue such and such".

peace

Event Timeline

Kipod raised the priority of this task from to Needs Triage.
Kipod updated the task description. (Show Details)
Kipod added a project: MediaWiki-JavaScript.
Kipod changed Security from none to None.
Kipod updated the task description. (Show Details)
Kipod subscribed.

of course, it might not be enough to teach MW this trick - in order to accomplish this you might need to teach gerrit to provide the "raw + patch",, and even do it in the correct mime-type (just like mw itself knows how to do when you tell it

?title=x&action=raw&ctype=whatever

if gerrit does not know how to do that, the gerrit team will probably be willing to teach it - the ability to do that can be useful elsewhere too.

peace.

i think the main difference here is that my suggestion has nothing to do with "spinning off" a whole new installation (locally or on a server). i am talking *only* about feeding the user's browser modified sources - MW does not even have to manage those sources - this is the job of gerrit (or git, if the patch is already checked-in to some branch).

T76245 is about "make it easier for developers to test a patch". my suggestion is about "make it _possible_ for a non-developer to test a patch"
(with known limitation that this can only work for purely JS/css patches).

peace.

Code for getting URL of script based on specific changeset can also use gitblit:

// example: getPatchsetRaw('mediawiki/core', 'a700f47e068d9eae0afb5efa6bac65ae0ef925e7', 'resources/mediawiki.special/mediawiki.special.createAccount.js')
function getPatchsetRaw(module, patchset, file) {
	return 'https://git.wikimedia.org/raw/'+encodeURIComponent(module)+'/'+patchset+'/'+encodeURIComponent(file);
}

Issues need to be considered:

  1. One have to know all the associated files of a patch to load it
  2. cross origin requests?
  3. it may be complicated to handle dependencies of mw.loader (some dependencies may be loaded before the "fix" is loaded?)
Aklapper triaged this task as Lowest priority.Dec 14 2014, 9:54 PM

How is this related to the MediaWiki-Javascript project?

Does this require changes in the MW core codebase itself?

As long as more or less everybody can propose a patch, this would be an obvious XSS-vector.

demon claimed this task.
demon subscribed.

As long as more or less everybody can propose a patch, this would be an obvious XSS-vector.

This.