Skip to content

Tags router: unset Itemid for not matching menu, in the end of build#40455

Merged
sdwjoomla merged 1 commit intojoomla:4.3-devfrom
Fedik:fix-40442-2
Apr 24, 2023
Merged

Tags router: unset Itemid for not matching menu, in the end of build#40455
sdwjoomla merged 1 commit intojoomla:4.3-devfrom
Fedik:fix-40442-2

Conversation

@Fedik
Copy link
Copy Markdown
Member

@Fedik Fedik commented Apr 22, 2023

Pull Request for Issue #40442 redo of #40448 .

Summary of Changes

Other way around,
Unset Itemid for not matching menu, in the end of Router::build.

Testing Instructions

Please follow #40442
Also open Tags list URL /component/tags/tags and check paginations links (Tag menu should be unpublished or not exists)

Actual result BEFORE applying this Pull Request

the url contain Itemid

Expected result AFTER applying this Pull Request

the url does not contain Itemid

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

@Fedik Fedik added bug Small A PR which only has a small change labels Apr 22, 2023
@ch2856
Copy link
Copy Markdown

ch2856 commented Apr 22, 2023

Tested, issue resolved, no itemid on pagination as well.

I didn't used the file it self, just changed line 185 since lines 155-157 brakes the site.

155-157 original:
$menuItem = $this->menu->getItem($query['Itemid']); if ($menuItem->query['option'] == 'com_tags') {

155-157 in the file here:
$menuItem = !empty($query['Itemid']) ? $this->menu->getItem($query['Itemid']) : false; if ($menuItem && $menuItem->query['option'] == 'com_tags') {

@richard67
Copy link
Copy Markdown
Member

@ch2856 Could you go to the issue tracker here https://issues.joomla.org/tracker/joomla-cms/40455 , then use the blue "Test this" button at the top left corner, and then select your test result (success as far as I understood) and finally submit? Thanks in advance.

@ch2856
Copy link
Copy Markdown

ch2856 commented Apr 22, 2023

I have tested this item ✅ successfully on 24e6776


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

@richard67
Copy link
Copy Markdown
Member

@ch2856 Thank you very much for testing.

@richard67
Copy link
Copy Markdown
Member

I have tested this item ✅ successfully on 24e6776


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

@richard67
Copy link
Copy Markdown
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 23, 2023
@richard67
Copy link
Copy Markdown
Member

I've remove the release blocker label because as far as I can see the pagination links had the Itemid query parameter also in 4.2.9. But there the pagination links were broken. On a clean, current 4.3-dev branch they work without this PR but still have the Itemid, and with this PR they don't have the Itemid and the links still work.

@ch2856
Copy link
Copy Markdown

ch2856 commented Apr 23, 2023

As far as I remember, before J4 we had no itemid using pagination. Since J4 the pagination for tags was broken so the solution here is the right one.

@richard67
Copy link
Copy Markdown
Member

As far as I remember, before J4 we had no itemid using pagination. Since J4 the pagination for tags was broken so the solution here is the right one.

@ch2856 As you can see I have successfully tested this PR, so I have no doubt that it is right and good. What I meant is that it's not a release blocker due to our definitions.

@sdwjoomla sdwjoomla added this to the Joomla! 4.3.1 milestone Apr 24, 2023
@sdwjoomla sdwjoomla merged commit fe8413e into joomla:4.3-dev Apr 24, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 24, 2023
@Fedik Fedik deleted the fix-40442-2 branch April 25, 2023 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Small A PR which only has a small change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants