? ? Pending

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
4 Jan 2018

Pull Request for Issue #19289 .

Summary of Changes

Do not lose menu item id in form action login.

Revert a part of code introduced in #19099: The URL from $active->params['login_redirect_url'] should not use association.

Testing Instructions

Configuration.

  1. Install multilingual joomla staging (with merged #19099) with (en-GB) as default for website and another language for a test user (ex. fr-FR).
  2. Create/use categories with menu association (Categories in EN, Categories in FR)
    • Categories in EN index.php?option=com_content&view=categories&id=0&Itemid=102&lang=en
    • Categories in FR index.php?option=com_content&view=categories&id=0&Itemid=104&lang=fr
  3. Create or use existed menu item for login view/form in com_users (/login-form) and set language en-GB
    No SEF address: index.php?option=com_users&view=login&Itemid=106&lang=en.

Test 1. PR enables association when SEF is off

  1. Turn off SEF.
  2. Set "Menu Item Login Redirect" to:
    • a menu item, ex "Categories in en_GB". (this should use menu item association)
    • Note: only internal URL should not use any association.
  3. Go to login form from users component - use above login menu item.
    (index.php?option=com_users&view=login&Itemid=106&lang=en)
  4. After login:
    Before PR you will go to ex index.php?option=com_content&view=categories&id=0&Itemid=102&lang=en.
    Association did not work.
    After PR you will go to ex index.php?option=com_content&view=categories&id=0&Itemid=104&lang=fr.
    Association works.
  5. IMO when the SEF is OFF, before PR, the whole association does not work.

Test 2. PR enables association when SEF is on

  1. Turn on SEF.
  2. Set "Menu Item Login Redirect" to:
    • a menu item, ex "Categories in en_GB". (this should use menu item association)
      Note: only internal URL should not use any association.
  3. Go to login form (ex index.php/en/login-form).
  4. After login:
    Before PR you will go to ex index.php/en/allcategories-en-gb.
    Association did not work.
    After PR you will go to ex index.php/fr/allcategories-fr-fr.
    Association works.

Test 3. Use redirection to preferred home page - issue mentioned by infograf768

  1. Do not use any login redirections in module mod_login.
  2. Go to Home in EN (index.php/en)
  3. Login by mod_login
  4. You will be (before and after PR) redirected to user language preferred home page index.php/fr

Test 4. Use association on active page

  1. Do not use any login redirections in module mod_login.
  2. Go to Categories in EN (index.php/en/allcategories-en-gb)
  3. Login by mod_login
  4. You will be (before and after PR) redirected to index.php/fr/allcategories-fr-fr
  5. You can set access to Registered to test association after log in too.
  6. --- No menu item ---
  7. Logout and go to /index.php/en/component/content/categories (no menu item)
  8. Login and check redirection: before and after PR you stay on the same page.

Test 5. Use association on module with "Login Redirection Page"

  1. Go to the module mod_login on backend and in configuration set option "Login Redirection Page" to menu item Categories in EN.
  2. Go to front/home page and log in:
    • Before PR you will be redirected to index.php/en/allcategories-en-gb (association does not work)
    • After PR you will be redirected to index.php/pl/allcategories-fr-fr (association works)

Test 6. Restricted article with "readmore" and enabled association on login page

  1. Follow the instruction at #19295 (comment)

Expected result

Association works.

Actual result

Association did not work in a few mentioned tests.

avatar csthomas csthomas - open - 4 Jan 2018
avatar csthomas csthomas - change - 4 Jan 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Jan 2018
Category Front End com_users Modules Plugins Templates (site)
avatar csthomas csthomas - change - 4 Jan 2018
Title
Revert to plugin system languageflter and fix form login action link
Undo the change in plugin system languageflter and correct the form action link for login
avatar csthomas csthomas - edited - 4 Jan 2018
avatar jsubri
jsubri - comment - 4 Jan 2018

I've updated my menu items with "Protostar Default" to benefit the debugging info, this is a bit odd:

with current Staging (3.8.4-dev) I'm not getting the stack dump , only "URL Invalid" from

throw new RouteNotFoundException('URL invalid');

Router.php at line 238 "URL invalid"
print_r gives:
$uri = Joomla\CMS\Uri\Uri Object ( [uri:protected] => index.php?Itemid=171&lang=fr-FR [scheme:protected] => [host:protected] => [port:protected] => [user:protected] => [pass:protected] => [path:protected] => p [query:protected] => Itemid=171&lang=fr-FR [fragment:protected] => [vars:protected] => Array ( [Itemid] => 171 [lang] => fr-FR ) )

Staging + 19295, I'm getting the stack dump with the same $uri:

1 | () | JROOT\libraries\src\Router\Router.php:238
2 | Joomla\CMS\Router\Router->parse() | JROOT\libraries\src\Router\SiteRouter.php:138
3 | Joomla\CMS\Router\SiteRouter->parse() | JROOT\plugins\system\languagefilter\languagefilter.php:639
4 | PlgSystemLanguageFilter->onUserLogin() |  
5 | call_user_func_array() | JROOT\libraries\joomla\event\event.php:70
6 | JEvent->update() | JROOT\libraries\joomla\event\dispatcher.php:160
7 | JEventDispatcher->trigger() | JROOT\libraries\src\Application\BaseApplication.php:108
8 | Joomla\CMS\Application\BaseApplication->triggerEvent() | JROOT\libraries\src\Application\CMSApplication.php:908
9 | Joomla\CMS\Application\CMSApplication->login() | JROOT\libraries\src\Application\SiteApplication.php:729
10 | Joomla\CMS\Application\SiteApplication->login() | JROOT\components\com_users\controllers\user.php:113
11 | UsersControllerUser->login() | JROOT\libraries\src\MVC\Controller\BaseController.php:710
12 | Joomla\CMS\MVC\Controller\BaseController->execute() | JROOT\components\com_users\users.php:15
13 | require_once() | JROOT\libraries\src\Component\ComponentHelper.php:381
14 | Joomla\CMS\Component\ComponentHelper::executeComponent() | JROOT\libraries\src\Component\ComponentHelper.php:356
15 | Joomla\CMS\Component\ComponentHelper::renderComponent() | JROOT\libraries\src\Application\SiteApplication.php:194
16 | Joomla\CMS\Application\SiteApplication->dispatch() | JROOT\libraries\src\Application\SiteApplication.php:233
17 | Joomla\CMS\Application\SiteApplication->doExecute() | JROOT\libraries\src\Application\CMSApplication.php:267
18 | Joomla\CMS\Application\CMSApplication->execute() | JROOT\index.php:49

Note: Itemid=171 relates to the "Menu Item Login Redirect".

Removing Login Redirect do not help, and getting:
Router.php at line 238 "URL invalid" and $uri = Joomla\CMS\Uri\Uri Object ( [uri:protected] => index.php?option=com_users&view=profile [scheme:protected] => [host:protected] => [port:protected] => [user:protected] => [pass:protected] => [path:protected] => p [query:protected] => option=com_users&view=profile [fragment:protected] => [vars:protected] => Array ( [option] => com_users [view] => profile ) )

However removing the "Associations" from the languagefilter plugin works like a charm. Guessing their is glitch somewhere.

avatar csthomas
csthomas - comment - 4 Jan 2018

Do you have ~ character in your test joomla address?

avatar jsubri
jsubri - comment - 4 Jan 2018

Do you have ~ character in your test joomla address?

No, and running on Windows :-(

Update on nailing down the issue: The error seems only triggered when I've "Yes" for both Automatic Language Change + Associations

image

working fine with:

image

working fine with:

image

working fine with:

image

avatar csthomas
csthomas - comment - 4 Jan 2018

Can you check what value of return field is send when you log in?

avatar csthomas
csthomas - comment - 4 Jan 2018

It would be good to close a browser in order to refresh cookies/session.

avatar infograf768
infograf768 - comment - 5 Jan 2018

Sorry folks, I still can't reproduce the original issue with Associations & Automatic Language Change
@jsubri Please explain in details how you get a pure http://localhost/staging/index.php/fr/?option=com_users&view=login as referrer. Also where you redirect to.
I suppose you have set Modern Routing for the User Manager

avatar jsubri
jsubri - comment - 5 Jan 2018

I've a Menu with a single entry , zero redirect used.
image

Both http://localhost/staging/index.php/fr/mon-acces and http://localhost/staging/index.php/fr/?option=com_users&view=login give me the below, their is no pure ?option=com_users&view=login used :

Wihout 19295
image

With 19265
image

I suppose you have set Modern Routing for the User Manager

Hoops, no it was on Legacy, turned to Modern, no change.

I've a workaround by turning off "Automatic Language Change"

I can't replicate with the Sample site !!! Not sure how I can help further.

Any idea how to trap/debug the below mentioned by @csthomas ?

Can you check what value of return field is send when you log in?

avatar csthomas
csthomas - comment - 5 Jan 2018

@jsubri
After you enabled Automatic Language Change + Associations and you got "URL Invalid".
Can you show me the value of "return" field from the source code of page where you are trying to log in.

Example <input type="hidden" name="return" value="aHR0cDovL2xvY2FsaG9zdC9zdGFnaW5nL2luZGV4LnBocC9lbi8=" />

The error you get is generated by unexpected value of $this->app->getUserState('users.login.form.return') at line 636

https://github.com/csthomas/joomla-cms/blob/fbc245b53371f5ad6b72cf9da781de2b227019e0/plugins/system/languagefilter/languagefilter.php#L636-L639

avatar jsubri
jsubri - comment - 5 Jan 2018

Staging 3.8.4-dev => "URL invalid"

<input type="hidden" name="return" value="MTcx" />
<input type="hidden" name="b003b985dd5244d286f183af8c98fcb9" value="1" />	

Staging 3.8.4-dev + patch 19295 => "URL invalid"

<input type="hidden" name="task" value="user.login" />
<input type="hidden" name="return" value="MTcx" />
<input type="hidden" name="82216a4fe3fcfee7a360b7023467385b" value="1" />

Staging 3.8.4-dev + languagefilter.php from 3.8.3 => WORKING FINE

<input type="hidden" name="return" value="MTcx" />
<input type="hidden" name="bbb71b793afa63f15f60897e9dfba795" value="1" />
avatar csthomas csthomas - change - 5 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 5 Jan 2018
avatar csthomas csthomas - change - 5 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 5 Jan 2018
avatar csthomas
csthomas - comment - 5 Jan 2018

@infograf768 I have updated the test instruction.

avatar infograf768
infograf768 - comment - 6 Jan 2018

Folks, as I said, I cannot reproduce the issue here.
My login form menu item(en-GB) with redirection to a Categories menu item(en-GB).

screen shot 2018-01-06 at 08 13 38

Then I log in frontend using the login form menu item:

login

All is fine.

avatar csthomas csthomas - change - 8 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 8 Jan 2018
avatar csthomas csthomas - change - 9 Jan 2018
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 9 Jan 2018
Category Front End com_users Modules Plugins Templates (site) Front End com_users Plugins
avatar csthomas csthomas - change - 9 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 9 Jan 2018
avatar csthomas
csthomas - comment - 9 Jan 2018

@jsubri I changed the code. Maybe this time it will work for you. I do not know why you previously had an exception, it may be related somehow to the Windows platform.

avatar jsubri jsubri - test_item - 9 Jan 2018 - Tested successfully
avatar jsubri
jsubri - comment - 9 Jan 2018

I have tested this item successfully on 57e5088

My specific login issue (multilingual + languagefilter) is resolved with Staging + this PR.

I'm tagging #19099 as I can't exclude a side effect from it. In case 19099 would be reverted this PR would need retesting or withdrawn.

Thank you


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

avatar infograf768 infograf768 - test_item - 10 Jan 2018 - Tested unsuccessfully
avatar infograf768
infograf768 - comment - 10 Jan 2018

I have tested this item ? unsuccessfully on 57e5088

ONCE MORE!!
This breaks Automatic Language Change.
Use the Login form Module with no redirect when the language is NOT the preferred user site language and you are not redirected to that language but remain on the same language.


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

avatar csthomas
csthomas - comment - 10 Jan 2018

At now I am writing a new test instruction and retesting everything once again.
After that I will add my results.

avatar csthomas csthomas - change - 10 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 10 Jan 2018
avatar csthomas csthomas - change - 10 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 10 Jan 2018
avatar csthomas csthomas - change - 10 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 10 Jan 2018
avatar csthomas
csthomas - comment - 10 Jan 2018

@infograf768 Please take a look at my tests. Remember to use fresh staging with merged #19099.
this PR also got fresh commit 57e5088.

@joomdonation You might be interested in the code review compared to 3.8.3 for languagefilter.php.
I added the result of the comparison at the top in the PR description.

avatar infograf768
infograf768 - comment - 11 Jan 2018

The problem is not solved.
User has French as preferred site language:
screen shot 2018-01-11 at 07 44 41
Default site language is English
screen shot 2018-01-11 at 07 45 07

The Login module has no redirection (uses Default).
The language filter Automatic Language Change is set to Yes
screen shot 2018-01-11 at 07 53 42

When the user logs in, for example on the English Home page, he should be redirected to the French Home page. It does NOT work.
login2

avatar csthomas
csthomas - comment - 11 Jan 2018

@infograf768 I have found the difference. I had an association between the home pages.
After I removed it I got your result.

avatar infograf768
infograf768 - comment - 11 Jan 2018

home pages do not need to be associated for this feature to work, and it should also work when they are associated. that is a basic core mulingual feature.

avatar csthomas csthomas - change - 11 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 11 Jan 2018
avatar csthomas
csthomas - comment - 11 Jan 2018

Now this should work.

avatar infograf768
infograf768 - comment - 12 Jan 2018

It breaks Register to Read More
login3

avatar infograf768
infograf768 - comment - 12 Jan 2018

@csthomas
I am thinking more and more that we have to revert #19099 (and evidently #19267)
as we are here going into issues (maybe only for @jsubri as I can't reproduce the problem).
And start again from fresh to solve #15730

@joomdonation @mbabker
What do you think?

Note: As it is now, without this PR, I could not find any error.

avatar csthomas
csthomas - comment - 12 Jan 2018

I can not reproduce.

Can you show me what URL do you have on "Read more"?
What is home page? Which menu item?
What is the link to article?
Do you have a menu item for login, which language?
What routing modern or stable for users/content component?

avatar infograf768
infograf768 - comment - 12 Jan 2018
  1. Home Menu item is a featured Menu item with Show Unauthorized Links set to Yes.
  2. The featured article is set to registered.
  3. The issue is the same if routing is set to modern or legacy
  4. The register to read more url is
    http://localhost:8888/installmulti/trunkgitnew/en/login-form?return=aW5kZXgucGhwP29wdGlvbj1jb21fY29udGVudCZ2aWV3PWFydGljbGUmaWQ9MTphcnRpY2xlLWVuLWdiJmNhdGlkPTgmbGFuZz1lbi1HQg==
  5. I do have some login form menu item with NO redirection set for each language, i.e. Default.
  6. Both login form menu items were associated, which created the issue. Before your PR, that association had no impact, which was the correct behaviour.
avatar csthomas
csthomas - comment - 12 Jan 2018

I tested J3.8.3, and the above example did not work in 3.8.3. It works in the same way as in this PR.

avatar infograf768
infograf768 - comment - 15 Jan 2018

@csthomas
This is possible, but before this PR (which again does not solve anything here afaik), that issue was solved, I guess by your former PRs post 3.8.3.

I will test again this PR asap.

avatar jsubri
jsubri - comment - 15 Jan 2018

@infograf768 , I'll try to re-produce the issue in the coming day(s) and I'll post back.

avatar csthomas
csthomas - comment - 15 Jan 2018

Yes, my previous PR solved that (but broke something else) and I reverted it.

I would like to leave it for another PR but if you ready want I will try do it on this PR.
I am worried that this may be a never-ending story.

I have some idea how to fix it but explain me what effect do you want. See on example below.

At start:

  1. Article(EN) does not have own menu item, it uses category(EN) menu item that has association to category(FR).
  2. As a guest I click on "read more" of article(EN)

Results/Solutions:

  1. Then I will be redirected to associated category-FR (NOT to article-FR) because article does not have own menu item. For me this is not the best behaviour.

  2. Back me to article-EN and do not use any association for "readmore" at all, when user has to login to see the article.

The general solution could be:

  • use return URL without Itemid if we do not need language redirection.
  • add Itemid to return URL if we want to use language redirection. Then you will be redirected to menu item association, not to the page association.
  • use login menu item (/login) with association to FR only if user go to direct login page, and do not use any associations if we go from readmore or mod_login.
avatar infograf768
infograf768 - comment - 16 Jan 2018

Article(EN) does not have own menu item, it uses category(EN) menu item that has association to category(FR).
As a guest I click on "read more" of article(EN)

Results/Solutions:
Then I will be redirected to associated category-FR (NOT to article-FR) because article does not have own menu item. For me this is not the best behaviour.

With or without your patch, logging to read more to get the full article does not redirect me to fr-FR but to the full en-GB article.

avatar csthomas
csthomas - comment - 16 Jan 2018

With or without your patch, logging to read more to get the full article does not redirect me to fr-FR but to the full en-GB article.

I assume that you does not have enabled association between login EN and login FR.

avatar csthomas
csthomas - comment - 16 Jan 2018

When you enable association between login pages then the result is different to above but the same as in 3.8.3.

Do you see any more issues, which has to be fixed in this PR?

avatar csthomas csthomas - change - 18 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 18 Jan 2018
avatar csthomas
csthomas - comment - 18 Jan 2018

@infograf768 I fixed your issue. Now it pass all six tests.

If both login pages are associated and "Menu Item Login Redirect" is not set (default) then redirection to login in FR does not work. To fix it admin has to set some menu item (to login page) in order to use association.

Please test.

avatar jsubri
jsubri - comment - 18 Jan 2018

UPDATE: key strokes send the message before I finished to type in. full content in the next entry

I created from scratch a site and I'm able to reproduce the "action link for login" that currently breaks in 3.8.4-dev (17-Jan-2018).
This PR solve it.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 18 Jan 2018

@jsubri please mark your Test as successfully:

  • open Issue Tracker
  • Login with your github-Account
  • Click on blue "Test this"-Button above Authors-Picture
  • mark your Test as successfully
  • hit "submit test result"
avatar jsubri
jsubri - comment - 18 Jan 2018

I have tested this item successfully on e9a5db0

I created from scratch a site and I'm able to reproduce the "action link for login" that currently breaks in 3.8.4-dev. If someone want a copy/backup of that site let me know.

There is still 1 issue for a URL that was working with 3.8.3 with modern router and breaking with staging (simple menu, registered, language ALL). As this is language all this can't be associated.
http://localhost/testing/index.php/fr/my-documents-registered-all-languages (OK)
http://localhost/testing/index.php/en/my-documents-registered-all-languages (404 when using the language switcher). "Remove IDs from URLs" is causing the issue, however adding #19280 on top of this PR clear the remaining issue.

Summary, this PR:

  • Solve the login issue.
  • Language Switcher tested successfully with: categories, articles, menu items,
  • PR 19280 to be merged to remove broken piece introduced in 3.8.4-dev

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19295.
avatar jsubri jsubri - test_item - 18 Jan 2018 - Tested successfully
avatar csthomas csthomas - change - 18 Jan 2018
Title
Undo the change in plugin system languageflter and correct the form action link for login
Correctly redirect after logging into the multilingual joomla with association enabled
avatar csthomas csthomas - edited - 18 Jan 2018
avatar infograf768
infograf768 - comment - 19 Jan 2018

I have tested this item ? unsuccessfully on e9a5db0

Login form menu items in both languages, not associated.
No redirection set.
Logging via the logging form menu item in the language which is NOT the user preferred language when Automatic change is set to Yes does NOT redirect to the other language as should.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19295.
avatar infograf768 infograf768 - test_item - 19 Jan 2018 - Tested unsuccessfully
avatar csthomas
csthomas - comment - 19 Jan 2018

Login form menu items in both languages, not associated.
No redirection set.
Logging via the logging form menu item in the language which is NOT the user preferred language when Automatic change is set to Yes does NOT redirect to the other language as should.

But it never worked.
If there is no redirection set and no association set for login page then it does not work.

When you enable association for login pages and:

  • set some redirection to ex. login page
  • or do some code changes like below:
diff --git a/components/com_users/controllers/user.php b/components/com_users/controllers/user.php
index 0912f4b664..934e8e63d2 100644
--- a/components/com_users/controllers/user.php
+++ b/components/com_users/controllers/user.php
@@ -92,7 +92,7 @@ class UsersControllerUser extends UsersController
                // Set the return URL if empty.
                if (empty($data['return']))
                {
-                       $data['return'] = 'index.php?option=com_users&view=profile';
+                       $data['return'] = JRoute::_('index.php?option=com_users&view=login', false);
                }
 
                // Set the return URL in the user state to allow modification by plugins

then it start working but only when association is enabled for login pages.

I can add this change to PR it will be helpful.

avatar csthomas
csthomas - comment - 19 Jan 2018

There are my all menu items
obraz

avatar jsubri
jsubri - comment - 19 Jan 2018

@csthomas , @infograf768 , happy to test the « login redirection based on user profile and/or association » on a separate pr.

avatar infograf768
infograf768 - comment - 19 Jan 2018

I have tested this item successfully on e9a5db0

OK. Let's go for it as we are now at RC.

Note: I still think that when there is no redirection set for the login forms, the user should be redirected to the Profile in his preferred site language. Let's say it is for another PR


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

avatar infograf768 infograf768 - test_item - 19 Jan 2018 - Tested successfully
avatar infograf768 infograf768 - change - 19 Jan 2018
Status Pending Ready to Commit
avatar infograf768
infograf768 - comment - 19 Jan 2018
avatar mbabker mbabker - change - 19 Jan 2018
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-01-19 17:44:27
Closed_By mbabker
Labels Added: ?
avatar mbabker mbabker - close - 19 Jan 2018
avatar mbabker mbabker - merge - 19 Jan 2018

Add a Comment

Login with GitHub to post a comment