[#32637] - Updated search.php to display proper title in search results
- Closed
- 12 Jun 2022
- Medium
- Build: staging
- # 32637
- Diff
- saumyasarkar11:patch-1
User tests: Successful: Unsuccessful:
Summary of Changes
Line 268 has been changed....
From: return preg_replace("/'"^/ui", '\1', $str);
To: return preg_replace("/([a-z])/ui", '\1', $str);
Testing Instructions
- Add an article whose title contains a word with double or single quotes (For eg: Typescript's Uses)
- Search a for the title
- Look out for the title of the search result
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_search |
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-03-11 05:24:41 |
| Closed_By | ⇒ | saumyasarkar11 | |
| Labels |
Added:
?
|
||
| Status | Closed | ⇒ | New |
| Closed_Date | 2021-03-11 05:24:41 | ⇒ | |
| Closed_By | saumyasarkar11 | ⇒ |
| Status | New | ⇒ | Pending |
| Title |
|
||||||
@saumyasarkar11 Now I found your other PR #32639 for the same issue with a different fix at the same place. Am confused. Which PR is the right one and which one shall be closed?
This one is the right one. I am closing the other one (PR #32639).
| Title |
|
||||||
| Title |
|
||||||
This is wrong...
preg_replace("/([a-z])/ui", '\1', $str);
The \1 is replacing the captured group, so replacing each a-z with the a-z itself.
The whole preg_replace is now redundant and you can just return $str;
The whole preg_replace is now redundant and you can just
return $str;
@saumyasarkar11 @PhilETaylor is right here. Could you change your pull request in this way? Thanks in advance.
| Labels |
Added:
?
|
||
Please review the new commit
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-04-08 09:01:53 |
| Closed_By | ⇒ | saumyasarkar11 | |
| Labels |
Removed:
?
|
||
| Status | Closed | ⇒ | New |
| Closed_Date | 2021-04-08 09:01:53 | ⇒ | |
| Closed_By | saumyasarkar11 | ⇒ |
| Status | New | ⇒ | Pending |
What's the status here? All my branches got deleted mistakenly and so this pull request @richard67
Dear maintainers, is this pr ready to test?
This PR would be better if there were Unit tests covering the method remove_accents that passed before this change and after this change.
and probably the removal of the @todo as well...as it makes no sense now
its also unclear from the screenshots what this PR actually resolves?
Thanks @PhilETaylor for your comment.
its also unclear from the screenshots what this PR actually resolves?
What i see the title is with pr yellow highlighted, without pr highlight stops after single quote.
What i see the title is with pr yellow highlighted, without pr highlight stops after single quote.
Ok I see that now in the screenshot - I guess I need better eyes :)
This PR will change the following when remove_accents is called.
-This is a" test
+This "is a" test
@saumyasarkar11 Please merge this unit test to your PR saumyasarkar11#3
| Category | Administration com_search | ⇒ | Administration com_search Unit Tests |
@saumyasarkar11 Please merge this unit test to your PR saumyasarkar11#3
Done
This has been pending for quite a while now. Any developments?
I will close here as the method does what its intended to do and this PR would change that behavior. There is also a seccond case here com_search has been removed from j4 and will nolonger be shipped with the core so i would like to keep the current behavior of this method for J3
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-06-12 18:41:19 |
| Closed_By | ⇒ | zero-24 | |
| Labels |
Added:
?
?
Removed: ? |
||
@saumyasarkar11 Now I found your other PR #32639 for the same issue with a different fix at the same place. Am confused. Which PR is the right one and which one shall be closed?