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

Missing URL information breaks menu alias routing #6254

Merged
merged 3 commits into from Mar 9, 2015

Conversation

@Hackwar
Copy link
Member

@Hackwar Hackwar commented Mar 2, 2015

This codeblock should actually be run on the process_before stage, since it expects only 2 query elements. If it is run on process_during, the languagefilter plugin already acted and added the language. If this is not executed, the alias menu item does not work correctly.

http://forum.joomla.org/viewtopic.php?f=711&t=875100

@zero-24
Copy link
Member

@zero-24 zero-24 commented Mar 2, 2015

@Hackwar Travis fail?

There was 1 failure:
1) JRouterSiteTest::testProcessBuildRules with data set #1 ('index.php?Itemid=42&test=true', 0, 'index.php?option=com_test&vie...mid=42')
JRouterSiteTest::testProcessBuildRules:699: value is not expected
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'index.php?Itemid=42&test=true'
+'index.php?option=com_test&view=test&Itemid=42'
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/cms/router/JRouterSiteTest.php:699b

https://travis-ci.org/joomla/joomla-cms/jobs/52708560

@Hackwar
Copy link
Member Author

@Hackwar Hackwar commented Mar 2, 2015

Yes, that is more or less expected. I wrote the unittests after the actual code and since that code was not correct, the unittests are not correct either.

@Hackwar
Copy link
Member Author

@Hackwar Hackwar commented Mar 2, 2015

Or rather, this shows bugs in the old code right now. There is more necessary than moving that part of the code a bit earlier.

@infograf768
Copy link
Member

@infograf768 infograf768 commented Mar 2, 2015

@test fails here
This creates other issues with the language switcher, SEF OFF.

A Classic menu item alias to the Home Page (Featured view) on top menu gives:
index.php?option=com_content&view=featured&Itemid=153&lang=it (instead of index.php?lang=it)
then when one wants to change language to display the home in another language by clicking on the flag, one gets for FR:
/index.php?lang=fr&Itemid=102&option=com_content

Hackwar added 2 commits Mar 6, 2015
@infograf768
Copy link
Member

@infograf768 infograf768 commented Mar 7, 2015

@test
We now have correct full links also through the language switcher, but still not a simple:
index.php?lang=en for the Home page

@fontanil
Copy link

@fontanil fontanil commented Mar 7, 2015

@test
When I apply this patch with patch tester, I get that: "Fatal error: Call to a member function getMode() on a non-object in /home/www/sitestests/multi/plugins/system/languagefilter/languagefilter.php on line 59"

@infograf768
Copy link
Member

@infograf768 infograf768 commented Mar 8, 2015

@fontanil
Try to also apply #6278

@fontanil
Copy link

@fontanil fontanil commented Mar 8, 2015

@test
Fine! both 6254 and 6278 patches solve language switcher URLs and language in Virtuemart popup.
Thanks!

@zero-24
Copy link
Member

@zero-24 zero-24 commented Mar 8, 2015

Moving to RTC here and set the relation to #6278


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

@infograf768 infograf768 added the RTC label Mar 9, 2015
@infograf768 infograf768 added this to the Joomla! 3.4.1 milestone Mar 9, 2015
wilsonge added a commit that referenced this pull request Mar 9, 2015
Missing URL information breaks menu alias routing
@wilsonge wilsonge merged commit bb7b378 into joomla:staging Mar 9, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zero-24 zero-24 removed the RTC label Oct 14, 2015
@Hackwar Hackwar deleted the Hackwar:patch-34 branch Jan 6, 2016
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

6 participants