Page MenuHomePhabricator

Make 'wp' prefix on the Edit form inputs configurable
Closed, DeclinedPublic

Description

It would be great if the 'wp' bit on EditPage form field ids was configurable. It could for example be replaced with $this->mPrefix which could be set to 'wp' in the EditPage constructor. This would allow extensions to change this value.

One practical instance where this would be useful is for an extension that I use that displays multiple edit forms on one page. Right now I have to modify these prefixes manually where ever they occur in EditPage so that the two edit forms don't contain overlapping ids.


Version: 1.13.x
Severity: enhancement
URL: http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/EditPage.php?view=markup

Details

Reference
bz15323

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 21 2014, 10:17 PM
bzimport set Reference to bz15323.
bzimport added a subscriber: Unknown Object (MLST).

Suggest WONTFIX. Per http://www.mediawiki.org/wiki/Manual:Coding_conventions#HTTP_and_session_stuff, the coding convention for all posted fields is to use the wp- prefix. Making this configurable this would be a bad idea, IMO.

If an extension uses the same name of a field that MW natively uses, the extension is creating problems with an otherwise functional convention.

I would very much like to see support for multiple edit forms. It has not to be the prefix that changes.

(In reply to comment #1)

Suggest WONTFIX. Per
http://www.mediawiki.org/wiki/Manual:Coding_conventions#HTTP_and_session_stuff,
the coding convention for all posted fields is to use the wp- prefix. Making
this configurable this would be a bad idea, IMO.

If an extension uses the same name of a field that MW natively uses, the
extension is creating problems with an otherwise functional convention.

It's not that my extension uses "wp..." field ids. My extension creates two EditPage objects and inserts them into the page[1]. Because the "wp"s are hardcoded, I get the same ids in both forms which is invalid HTML. My options are 1) modify the EditPage class to get rid of the hardcoding, which is what I currently do, 2) capture the output and grep_replace the prefixes or 3) make my own form class which duplicates large chunks of the current EditPage class. None of these options are ideal. EditPage could be made more reusable and extensible if the "wp" were a prefix that could be redefined. Then you could basically make an unlimited number of EditPage objects each with a different prefix, and stick them in a page.

Also, regardless of my specific extension, its good practice to make prefixes (or namespace or whatever) configurable so you don't have to do a massive search/replace to reuse the code outside of MediaWiki.

[1] It essentially lets you view and edit two related pages at once. Imagine for example that you wanted to edit a page and its talk page on the same screen.

mattj wrote:

The prefix cannot be changed, as it is a coding standard and would make the form inconsistent. Would a suffix work instead (e.g. wpFieldName-2)? If so, that wouldn't be hard to add (simply an optional parameter or member variable), and if it'll do, i'll code that in for you.

(In reply to comment #4)

The prefix cannot be changed, as it is a coding standard and would make the
form inconsistent. Would a suffix work instead (e.g. wpFieldName-2)? If so,
that wouldn't be hard to add (simply an optional parameter or member variable),
and if it'll do, i'll code that in for you.

Sure it can be changed. Just replace all instances of wp with $this->mPrefix. Then in the EditPage constructor set $this->mPrefix = 'wp'. You get the exact same default behavior, but a subclass could set $this->mPrefix = 'whatever'. So for example line 554:

$this->section = $request->getVal( 'wpSection', $request->getVal( 'section' ) );

would become:

$this->section = $request->getVal( $this->mPrefix.'Section', $request->getVal( 'section' ) );

Actually, the prefix is hardcoded in other places as well like the javascript, so a subclass couldn't really change it that easily. But its a start.

The next step would be to split the HTML generation functions out of EditPage into a proper template. Then you could move all the prefix junk into the HTML generation functions and the $request object and you would never have to worry about prefixes again. Sigh, one day...

mattj wrote:

I believe it could be done very easily, however it should not be done, as it would make the codebase inconsistent (everywhere else uses wp as a prefix, it helps keep things organised). A suffix would do the job just as well, and we can keep the codebase consistent, while still allowing several of these forms onto the page.

(In reply to comment #1)

Suggest WONTFIX. Per
http://www.mediawiki.org/wiki/Manual:Coding_conventions#HTTP_and_session_stuff,
the coding convention for all posted fields is to use the wp- prefix. Making
this configurable this would be a bad idea, IMO.

If an extension uses the same name of a field that MW natively uses, the
extension is creating problems with an otherwise functional convention.

That page is full of errors, and this is one of them. Prefixes such as wp and ws are a holdover from register_globals. They are not necessary in new code and are not uniformly used.

It is, however, quite reasonable to want to have unique ID attributes in separate invocations of EditPage. But the implementation Christian suggests is certainly not the ideal one. For one thing, it should be a suffix, not a prefix. I suggest you request that feature on a new bug report without suggesting a particular implementation.