Skip to content

[Regression] Keep B/C in the redirect plugin#17124

Merged
rdeutz merged 3 commits intojoomla:stagingfrom
zero-24:redirect_bc
Jul 17, 2017
Merged

[Regression] Keep B/C in the redirect plugin#17124
rdeutz merged 3 commits intojoomla:stagingfrom
zero-24:redirect_bc

Conversation

@zero-24
Copy link
Copy Markdown
Contributor

@zero-24 zero-24 commented Jul 13, 2017

Summary of Changes

If you have set up some urls with mixed upper and lower case in the redirect component before this patch they did not got redirected anymore.

Testing Instructions

  • Set up a redirect with a uppercase char in the URL.
  • call the exact URL with the uppercase
  • no redirect
  • apply this patch
  • it is working again

Expected result

Old redirects still work

Actual result

Old redirects with uppercase chars are broken

Documentation Changes Required

None

@wilsonge
Copy link
Copy Markdown
Contributor

that patch

Which patch xD

@zero-24
Copy link
Copy Markdown
Contributor Author

zero-24 commented Jul 13, 2017

Which patch xD

that => this

So without this patch here they did not got redirected anymore. I'm going to fix that ;)

@zero-24
Copy link
Copy Markdown
Contributor Author

zero-24 commented Jul 13, 2017

This patch here: #13853 introduced the issue by forcing it to be lowercase but not respecting the mixed ones in the database. ;)

@ghost
Copy link
Copy Markdown

ghost commented Jul 14, 2017

@zero-24 i setup following redirect which works without PR so i guess i didn't get Issue:
bildschirmfoto 2017-07-14 um 07 10 48

@infograf768
Copy link
Copy Markdown
Member

hmm, This here works without patch:

screen shot 2017-07-14 at 09 07 38

@zero-24
Copy link
Copy Markdown
Contributor Author

zero-24 commented Jul 14, 2017

The problem is that they does not find the old URL.

Please set the old URL withe upper cases and a different URL with lower cases. Please also make sure that the plugin is enabled.

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jul 14, 2017

When the Expired URL contains a some Upper case, indeed this patch corrects the issue.
screen shot 2017-07-14 at 09 36 44


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

@infograf768
Copy link
Copy Markdown
Member

I have tested this item ✅ successfully on 6edabc8


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

@zero-24
Copy link
Copy Markdown
Contributor Author

zero-24 commented Jul 14, 2017

When the Expired URL contains a some Upper case, indeed this patch corrects the issue.

Is'n this than a succesful test?

@zero-24
Copy link
Copy Markdown
Contributor Author

zero-24 commented Jul 14, 2017

Ah our messages crossed ;)

@zero-24 zero-24 added this to the Joomla 3.7.4 milestone Jul 14, 2017
@matrikular
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 031a3d8


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

1 similar comment
@infograf768
Copy link
Copy Markdown
Member

I have tested this item ✅ successfully on 031a3d8


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.7.4 milestone Jul 17, 2017
@infograf768
Copy link
Copy Markdown
Member

RTC, good for 3.7.4


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 17, 2017
@infograf768 infograf768 added this to the Joomla 3.7.4 milestone Jul 17, 2017
@infograf768 infograf768 assigned infograf768 and rdeutz and unassigned infograf768 Jul 17, 2017
@rdeutz rdeutz merged commit 11432b5 into joomla:staging Jul 17, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 17, 2017
@zero-24 zero-24 deleted the redirect_bc branch July 17, 2017 08:28
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.

6 participants