Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing input filter to tags #15950

Merged
merged 2 commits into from
May 23, 2017
Merged

Add missing input filter to tags #15950

merged 2 commits into from
May 23, 2017

Conversation

zero-24
Copy link
Contributor

@zero-24 zero-24 commented May 10, 2017

Summary of Changes

Missing input filter to tags. cc @dgt41

Testing Instructions

Apply this patch and confirm you can still can use the frontend tags seach

Expected result

Still works

Actual result

Works but without input filter

Documentation Changes Required

None

@dgrammatiko
Copy link
Contributor

I have tested this item ✅ successfully on 0aa358b


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

@brianteeman
Copy link
Contributor

does this mean that all uses of 'integer' need to be changed to 'int' ??

@mbabker
Copy link
Contributor

mbabker commented May 11, 2017

does this mean that all uses of 'integer' need to be changed to 'int' ??

Actually, no... https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/filter/input.php#L150-L151

@brianteeman
Copy link
Contributor

Now I am more confused why it is being changed here. Guess I have lots to learn

@zero-24
Copy link
Contributor Author

zero-24 commented May 11, 2017

Now I am more confused why it is being changed here. Guess I have lots to learn

I have changed it based on the doc block: https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/filter/input.php#L116 As Michael pointed we don't need to change that but we should be consistend there i think.

@ghost
Copy link

ghost commented May 23, 2017

Installing PR failed:
bildschirmfoto 2017-05-23 um 15 22 21

System information

3.8-dev
Multilanguage Site
macOS Sierra, 10.12.4
Firefox 53 (64-bit)

MAMP 4.1.1

  • PHP 7.0.15
  • MySQLi 5.6.35

@mbabker
Copy link
Contributor

mbabker commented May 23, 2017

Then you're not on the right Joomla version. That file most assuredly exists in 3.7 (staging), with the namespacing effort it doesn't exist in 3.8.

@brianteeman
Copy link
Contributor

Always test on staging unless it says it is for a different version

@ghost
Copy link

ghost commented May 23, 2017

Thanks for Info. I updated from 3.7.3-dev cause got Message to update.

bildschirmfoto 2017-05-23 um 15 52 03

@ghost
Copy link

ghost commented May 23, 2017

I have tested this item ✅ successfully on 0aa358b

Using PR and search for Tag "red" got:
bildschirmfoto 2017-05-23 um 15 55 20


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

@ghost
Copy link

ghost commented May 23, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 23, 2017
@zero-24 zero-24 added this to the Joomla 3.7.3 milestone May 23, 2017
@rdeutz rdeutz merged commit 92ea2ea into joomla:staging May 23, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 23, 2017
@zero-24 zero-24 deleted the inputfilter branch May 23, 2017 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants