[#45947] - [6.0] Fix incorrect language tag comparison
- Pending
- Medium
- Build: 6.0-dev
- # 45947
- Diff
- Fedik:fix-45941
User tests: Successful: Unsuccessful:
Pull Request for Issue #45941 and #45945 .
Summary of Changes
Fix language tag comparison when short tag from URI was compared with full tag.
Testing Instructions
It tricky to test in the core.
Add dd('test'); after
https://github.com/joomla/joomla-cms/blob/ea20ffd58058bc4fce3e547ab30a7d7903369306/plugins/system/sef/src/Extension/Sef.php#L113-L112
And open /index.php?option=com_content&view=article&id=6&lang=en on testing installation.
Actual result BEFORE applying this Pull Request
PHP warnings.
Expected result AFTER applying this Pull Request
No PHP warnings
Link to documentations
Please select:
- Documentation link for docs.joomla.org:
- No documentation changes for docs.joomla.org needed
- Pull Request link for manual.joomla.org:
- No documentation changes for manual.joomla.org needed
@Hackwar please check, maybe the correct full tag should be already set in parseRule?
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End Plugins |
| Labels |
Added:
bug
PR-6.0-dev
|
||
I have tested this item ✅ successfully on 89b46af
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45947.
@Fedik Yes good catch, this seems to fix the problem
(sorry for my question now but does that mean what I found is nothing to be concerned about, i.e. that I can validate your PR as is or am I waiting for an update from your to re-test and where I should not come across the same thing (HTTPS not being able to be turned on)?
Thanks for your patience - just trying to make sure @Fedik
Do you know which pr introduced this bug? We should check everything in that pr
Here still a question, should the full tag be already set in the parseRule method?
Hard to compare with old behavior.
I have tested this item ✅ successfully on 89b46af
I have tested it successfully...
That said, I also found this artifact/quirk discussed here:
#45947 (comment)
based on @Fedik's replies and comments and an upcoming future review by Hackwar, I will validate my test.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45947.
Do not set it RTC for now, I still need to check few things.
Okay, should be good now,
I checked Joomla 5 does not set lang in to Uri for full tag, and I update the pr to work the same.
I have tested this item ✅ successfully on cde2124
I have retested this successfully. Thanks @Fedik!
(the HTTPS quirk that I mentioned in my earlier test is still present if it matters).
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45947.
| Labels |
Added:
PBF
|
||
I have tested this item ✅ successfully on 9742ae1
I have re-tested this successfully during PBF 23.08. Thanks @Fedik!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45947.
@joeforjoomla Could you test this PR again? Thanks in advance.
@Fedik Thanks for the discovery and fix. I was going to pass it as successful but then I discovered a really odd situation... let me explain...
This testing installation (which I recently re-installed) did NOT have the System Configuration setting:
Force HTTPS turned on... (Entire Site)
During testing I found that WITH your patch on and the dd('test'); text added to line 113, I could not turn the site to HTTPS... I would get:
HTTPS has not been enabled as it is not available on this server. HTTPS connection test failed with the following error: HTTPS version of the site returned an invalid HTTP status code.
I have tried this now 5 times so I doubt it's my testing method/process. I also have other test installations on the same domain in other sub-folders and none of them have an issue with Enabling/Disabling HTTPS...
Any ideas as this relates? is it important/worrysome or just because of the specific testing done for this PR?
Thanks.