Skip to content

Conversation

@conseilgouz
Copy link
Contributor

@conseilgouz conseilgouz commented Mar 24, 2023

Pull Request for Issue #40182 .

Summary of Changes

In version 4.2.9, $this->router->app is being used instead of $this->app.

Testing Instructions

Use Joomla 4.3 nightly build
Create at least one article
Create an instance of Articles - Latest module
Change Home menu item to link to Featured Contacts menu item type. If you have any menu items link to com_content before, please unpublish it or make them 'hidden'
Try to access to an article from Articles - Latest module above

Actual result BEFORE applying this Pull Request

ERROR 0 - Call to a member function getInput() on null in ...\libraries\src\Component\Router\Rules\NomenuRules.php:82

Expected result AFTER applying this Pull Request

No error

@joomdonation
Copy link
Contributor

Testing Instructions

  1. Use Joomla 4.3 nightly build
  2. Create at least one article
  3. Create an instance of Articles - Latest module
  4. Change Home menu item to link to Featured Contacts menu item type. If you have any menu items link to com_content before, please unpublish it.
  5. Try to access to an article from Articles - Latest module above

Actual result BEFORE applying this Pull Request

You get fatal error: ERROR 0 - Call to a member function getInput() on null in ...\libraries\src\Component\Router\Rules\NomenuRules.php:82
$this->app is not initialized

Expected result AFTER applying this Pull Request

No error. The article is being displayed properly.

@joomdonation
Copy link
Contributor

Oh, No. You are making more changes than needed. All you need to do is change line 82 from:

$input = $this->app->getInput();

To

$input = $this->router->app->getInput();

@conseilgouz
Copy link
Contributor Author

No, bug has been introduced by #39029
So, input() lines have to become getInput().

@conseilgouz
Copy link
Contributor Author

conseilgouz commented Mar 24, 2023

Just updating line 82 causes line 83 crash : ERROR 0 - Object of type Joomla\CMS\Input\Input is not callable.....

@joomdonation
Copy link
Contributor

Line 83 crash because you also made change to that line. If you just make change to line 82 as I mentioned, it will work well

@conseilgouz
Copy link
Contributor Author

conseilgouz commented Mar 24, 2023

Line 83 crash because you also made change to that line. If you just make change to line 82 as I mentioned, it will work well

No, original line 83 contains if ($view->parent_key && $input->get($view->parent_key)) {

If I use your line, it crashes as $input is a method and not an object.

@conseilgouz
Copy link
Contributor Author

conseilgouz commented Mar 24, 2023

Lines 83 to 85 have been updated to be consistent with pr #39029

@joomdonation
Copy link
Contributor

If I use your line, it crashes as $input is a method and not an object.

$input is an object. Not method. Like 82, if you make change as mentioned, will return application input object

$input = $this->router->app->getInput();

$this->router->app returns the application object. And calling $this->router->app->getInput(); will return input object, not a method. You can follow the testing instructions which provided to test the result yourself.

@conseilgouz
Copy link
Contributor Author

I tried PR as we discussed :

  1. just updating line 82 to $input = $this->router->app; works, with no other change (but it is not consistent with PR [4.3] Change application input access to getInput #39029 )
  2. Updating line 82 as you suggested causes an error on line 83

@joomdonation
Copy link
Contributor

Your code does not work. I applied the changes (which I know it's wrong) and see some notices and 404 not found error:

Notice
: Object of class Joomla\CMS\Input\Input could not be converted to float in
libraries\vendor\joomla\database\src\Mysqli\MysqliStatement.php
on line
435

Notice
: Object of class Joomla\CMS\Input\Input could not be converted to int in
\libraries\vendor\joomla\database\src\Mysqli\MysqliStatement.php
on line
435

@conseilgouz
Copy link
Contributor Author

conseilgouz commented Mar 24, 2023

I just tried following your testing instructions and, indeed, your single line works.
But, when I try it with Phocadownload component, it blows up.
May be a phocadownload issue.
I'm doing some more research on this.

Make it work according testing instructions
@joomdonation
Copy link
Contributor

I have tested this item ✅ successfully on 6c948fa


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

@conseilgouz
Copy link
Contributor Author

For information, hidden components behave like no-menu components
After some more testing, I was not able to reproduce error on line 83 with other components (including phocadownload). I must have dreamt it...
So this PR only contains one line.

@conseilgouz conseilgouz changed the title fix #40182 [4.3] fix #40182 Mar 24, 2023
@conseilgouz conseilgouz changed the title [4.3] fix #40182 [4.3] fix for #40182 Mar 24, 2023
@conseilgouz conseilgouz changed the title [4.3] fix for #40182 [4.3] fix for #40182 : error in router in NomenuRules Mar 24, 2023
@conseilgouz conseilgouz changed the title [4.3] fix for #40182 : error in router in NomenuRules [4.3] fix for #40182 : Router error in NomenuRules Mar 24, 2023
@sdwjoomla
Copy link
Contributor

I have tested this item ✅ successfully on 6c948fa


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

@Quy
Copy link
Contributor

Quy commented Mar 24, 2023

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 24, 2023
@obuisard obuisard added this to the Joomla! 4.3.0 milestone Mar 24, 2023
@obuisard obuisard merged commit 9af564d into joomla:4.3-dev Mar 24, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 24, 2023
@obuisard
Copy link
Contributor

Thank you @conseilgouz for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants