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

Removing unnecessary code from com_search router #12482

Merged
merged 2 commits into from Aug 22, 2017
Merged

Conversation

@Hackwar
Copy link
Member

@Hackwar Hackwar commented Oct 19, 2016

The code that is being removed does nothing. Literally. It acts on an empty array... I was thinking about removing much of the code in parse(), too, but while Joomla itself does not create such URLs, you could hand in the searchword as a first segment and that worked in the past, so we will have to keep that in until at least 4.0...

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Oct 20, 2016

Looks like this was introduce by @wilsonge here #3399

@SamuelSchepp
Copy link

@SamuelSchepp SamuelSchepp commented Aug 22, 2017

I have tested this item successfully on 6531a19

Removed code does nothing, since $segments is always array().
Searching by SEF URL index.php/search/my%20word/ still works.
@icampus


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

@eXsiLe95
Copy link
Contributor

@eXsiLe95 eXsiLe95 commented Aug 22, 2017

I have tested this item successfully on 6531a19

Tested

Search

  1. Go to /index.php/search
  2. Search for some content
  3. Go to /index.php/search/<some%20content> (which is a SEF URL)
  4. Activate bugfix
  5. Go to /index.php/search
  6. Search for some content
  7. Go to /index.php/search/<some%20content> (which is a SEF URL)

Behaviour

Removed code does not change anything

Comparing the results of the test (step 1-3 and 5-7), there is no difference. The usual search as well as SEF URLs still work. The removed code does not affect anything since $segments is always an empty array.

Tested @icampus


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

@ghost
Copy link

@ghost ghost commented Aug 22, 2017

RTC after two successful tests.

@mbabker mbabker added this to the Joomla 3.8.0 milestone Aug 22, 2017
@mbabker mbabker merged commit 4f23f2e into joomla:staging Aug 22, 2017
3 of 4 checks passed
3 of 4 checks passed
continuous-integration/jenkins/pr-merge This commit cannot be built
Details
JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Hackwar Hackwar deleted the Hackwar:patch-11 branch May 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants