? Success

User tests: Successful: Unsuccessful:

avatar Achal-Aggarwal Achal-Aggarwal - open - 14 Mar 2014
avatar Achal-Aggarwal
Achal-Aggarwal - comment - 14 Mar 2014
avatar vdespa
vdespa - comment - 15 Mar 2014

I don't quite see what this PR fixes, as there are no issues with the code.

avatar Achal-Aggarwal
Achal-Aggarwal - comment - 15 Mar 2014

@vdespa Following are the lines in which either $ is before name ($this->name) or is present before protected property ($this->$accept).

  1. https://github.com/joomla/joomla-cms/pull/3306/files#diff-dec67eea53cfde023707fca26cc69a58L74
  2. https://github.com/joomla/joomla-cms/pull/3306/files#diff-bad19b858a25cd6463f9c08e6c011b1aL100
  3. https://github.com/joomla/joomla-cms/pull/3306/files#diff-263feb5d955c0931bea2544b36815f7fL84 (issue reported in 33468)

https://github.com/joomla/joomla-cms/pull/3306/files#diff-037720088ad5e312e2824e27f2cd5550L101 In this inputmode gets a string value instead of boolean.

Other changes are because there is only one property which is present in overridden set & get. So I thought to not to use $name.

avatar vdespa
vdespa - comment - 15 Mar 2014

The only issue was in JFormFieldTextarea, which already has a PR.

IMHO you are not actually fixing anything, nor are you breaking anything.

avatar Achal-Aggarwal
Achal-Aggarwal - comment - 15 Mar 2014

The issue Frederic noticed is same as with meter property of password and accept of file field. Try doing the same thing with these fields as described in the joomlacode tracker. Or apply this diff https://gist.github.com/Achal-Aggarwal/9563815#file-typo-password-field-test and run phpunit tests/unit/suites/libraries/joomla/form/fields/JFormFieldPasswordTest.php

avatar brianteeman
brianteeman - comment - 8 Aug 2014

closed as per the comment on the tracker

avatar brianteeman brianteeman - change - 8 Aug 2014
Status New Closed
Closed_Date 0000-00-00 00:00:00 2014-08-08 14:36:04
avatar brianteeman brianteeman - close - 8 Aug 2014
avatar wilsonge
wilsonge - comment - 8 Aug 2014

@Achal-Aggarwal the password and file field bugs fixed here still exist. would you mind doing a new pr merging in master to fix please? (also as you have a unit test there would be awesome to add that in!)

avatar Achal-Aggarwal
Achal-Aggarwal - comment - 12 Aug 2014

@wilsonge test PR 4100.

Add a Comment

Login with GitHub to post a comment