? ? Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
2 Oct 2017

Numerous options in the modules are for numbers but the xml field type is text. This pr changes them to the correct value of number.

This ensures that a user cannot enter a letter and only numbers. There are no b/c issues as only a numerical character could ever have worked and there is no change in the way the data is stored.

I spotted this when debugging a user's site where they put "five" in the count field instead of "5" and wondered why it didnt work. After this PR that isnt possible

avatar joomla-cms-bot joomla-cms-bot - change - 2 Oct 2017
Category Modules Administration Front End
avatar brianteeman brianteeman - open - 2 Oct 2017
avatar brianteeman brianteeman - change - 2 Oct 2017
Status New Pending
af71c95 2 Oct 2017 avatar brianteeman fix
avatar brianteeman brianteeman - change - 2 Oct 2017
Labels Added: ?
avatar Quy Quy - test_item - 2 Oct 2017 - Tested successfully
avatar Quy
Quy - comment - 2 Oct 2017

I have tested this item successfully on af71c95

Code review


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18194.

avatar chmst chmst - test_item - 2 Oct 2017 - Tested unsuccessfully
avatar chmst
chmst - comment - 2 Oct 2017

I have tested this item ? unsuccessfully on af71c95

Tested successfully on some modules, but unsuccessfully on modules/mod_articles_category/mod_articles_category

Additionally there could be used a filter="integer".


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18194.

avatar brianteeman
brianteeman - comment - 2 Oct 2017

@chmst can you give some more information please as I cannot tell from your test what doesnt work

avatar chmst
chmst - comment - 2 Oct 2017

You have for the Field count type="numbertext". It doenst work, of course. only a typo :) , but should not be merged.

avatar brianteeman
brianteeman - comment - 2 Oct 2017

That was already fixed af71c95

avatar chmst
chmst - comment - 2 Oct 2017

This was not visible when I startet testing. So I can repeat the tests.

avatar chmst chmst - test_item - 2 Oct 2017 - Tested successfully
avatar chmst
chmst - comment - 2 Oct 2017

I have tested this item successfully on af71c95

Repeated some tests and inspection.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18194.

avatar franz-wohlkoenig franz-wohlkoenig - change - 3 Oct 2017
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 3 Oct 2017

RTC after two successful tests.

avatar infograf768
infograf768 - comment - 3 Oct 2017

@mbabker
3.8.1 ?

avatar brianteeman
brianteeman - comment - 3 Oct 2017

There is no rush to get this merged. Today I will be reviewing components and plugins for the same

avatar infograf768
infograf768 - comment - 3 Oct 2017

See my comment here:
#18199 (comment)

avatar brianteeman
brianteeman - comment - 3 Oct 2017

See my reply #18199 (comment)

avatar mbabker mbabker - change - 5 Oct 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-10-05 12:15:12
Closed_By mbabker
Labels Added: ?
avatar mbabker mbabker - close - 5 Oct 2017
avatar mbabker mbabker - merge - 5 Oct 2017
avatar brianteeman
brianteeman - comment - 5 Oct 2017

Thanks

Add a Comment

Login with GitHub to post a comment