Page MenuHomePhabricator

HTMLForm changes broke stashed uploads
Closed, ResolvedPublic

Description

Author: Bryan.TongMinh

Description:
HTMLForm changes broke stashed uploads because wpSourceType is double prefixed with wp. Need to find out where.


Version: 1.17.x
Severity: major

Details

Reference
bz26613

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:15 PM
bzimport set Reference to bz26613.

Bryan.TongMinh wrote:

To reproduce, upload a file that does raise a warning and click "ignore and submit file anyway button"

Bryan.TongMinh wrote:

In 1.16 hidden fields were unconditionally not prefixed with wp, whereas in 1.17 in certain cases they are, breaking backwards compatibility. It kinda sucks to break an interface after only a single release.

happy.melon.wiki wrote:

$form = new HTMLForm( array(
'Text' => array(

		'type' => 'text',
		'default' => 'foo',

),
'TextOverridden' => array(

		'type' => 'text',
		'name' => 'DifferentTextOverridden',
		'default' => 'foo',

),
'ArrayName' => array(

		'type' => 'hidden',
		'default' => 'foo',

),
'ArrayNameOverridden' => array(

		'type' => 'hidden',
		'name' => 'DifferentArrayNameOverridden',
		'default' => 'bar',

),
) );
$form->addHiddenField( 'FunctionName','baz' );
$form->setTitle($this->getTitle());
$form->displayForm('');

1.16.1

<input name="wpText">
<input name="wpTextOverridden">
<input type="hidden" name="FunctionName">
<input type="hidden" name="ArrayName">
<input type="hidden" name="ArrayNameOverridden">

trunk

<input name="wpText">
<input name="DifferentTextOverridden">
<input type="hidden" name="FunctionName">
<input type="hidden" name="wpArrayName">
<input type="hidden" name="DifferentArrayNameOverridden">

The main change in 1.17 was to introduce the 'name' override: if you specified 'name' => 'Foo' in the descriptor array, that would be used unconditionally, and without any modification. That applies to all types of fields. Given that, there's no reason to special-case hidden fields *except* for B/C. And as I said on IRC, I'd rather have it consistent and transiently incompatible, than inconsistent forever.

Specific issue raised was fixed in r79758.