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

[4.0] [RFC] Should routing changes from 3.8.4 remain in 4.0 or should be reverted too? #19537

Closed
csthomas opened this issue Feb 2, 2018 · 30 comments
Labels

Comments

@csthomas
Copy link
Contributor

csthomas commented Feb 2, 2018

I always want joomla URLs to be more deterministic.

The full link (with option = ...) should not depend on which page it was generated on.

If you create a link without the view (or other required parameter is missing) like JRoute::_('index.php?option=com_content') then menu item won't be found.

  1. Itemid should not be inherited for full link. Because of that some link will look like /component/content/article/1-category-without-menu-item/1-article-and_category-tree-without-menu-item

  2. Partial link like index.php?view=article&id=1, &start=2 or only index.php have to inherit Itemid as it was before.

These two rules I underscored in version 3.8.4 but it seems it was too early or should not happen.

The main question

Can we stay with these changes in version 4.0 or we have to revert it too?

@brianteeman
Copy link
Contributor

Could you explain the benefit of the change you propose

@alessio-gaggii
Copy link

I feel like I should inform you that this change made in the version 3.8.4 broke hundreds, if not thousands, of websites that were using third party extensions.
In fact, any call to JRoute::_('index.php?option=com_customcomponent') so without &Itemid=n, no longer rewrites the URL correctly, but becomes /component/customcomponent/ instead. The current Itemid of the request is now ignored.
This would be a total disaster in terms of backward compatibility with third party extensions. Forums of extensions developers are literally exploding in these days after the release of Joomla 3.8.4.
I really hope you will take the right decision.

@ghost
Copy link

ghost commented Feb 2, 2018

@alessio-gaggii thanks for Comment, its a knowing Issue, for Example #19512.

@infograf768
Copy link
Member

@csthomas

Can we stay with these changes in version 4.0 or we have to revert it too?

As it will be reverted for 3.8.5 (coming soon), anything similar already merged into 4.0 should imho be reverted.

@csthomas
Copy link
Contributor Author

csthomas commented Feb 3, 2018

@infograf768 OK, it will easier to merge 4.0 with 3.8.5.

If I find enough energy, I will try to create a new PR at 4.0 and I will describe it better.

@Bakual
Copy link
Contributor

Bakual commented Feb 3, 2018

Few points:

  • An URL generated by JRoute should always (ALWAYS, without exception) have an Itemid.
  • If JRoute can't find a matching Itemid for the link but the currently active Itemid is from the same component, then the active one is a better match than the homepage and should be used. We should always try to use the best matching menuitem.

@csthomas
Copy link
Contributor Author

csthomas commented Feb 3, 2018

An URL generated by JRoute should always (ALWAYS, without exception) have an Itemid.

If you go directly to page /component/content/article/1-article then that page does not have any Itemid in joomla 3.8.3 with single language. This is a behaviour of all joomla versions.

If JRoute can't find a matching Itemid for the link but the currently active Itemid is from the same component, then the active one is a better match than the homepage and should be used. We should always try to use the best matching menuitem.

Hmm,
this is an example of stable routing, but modern routing has similar problems.

Assume that I installed fresh joomla 3.8.3 (single language) and created 3 top menu items:

  1. Home page - featured view of articles
  2. Login page - com_users/login
  3. "Archive" menu item for archived articles

Then I created a few categories and articles.

  1. Articles has address like

    • /2-category/20-article
    • /3-category/30-article
    • /4-category/40-article
  2. Archived article may have links (generated by joomla) like:

    • /1-category/10-old-article
    • /archive/1-category/10-old-article

Now I create a module to display "top 10 news" and another for archive articles.
When I go to /archive page then link to my "top 10 news" also contains /archive prefix.
Now links generated by joomla are:

  • /archive/2-category/20-article
  • /archive/3-category/30-article
  • /archive/4-category/40-article

If we change home page to login page then link to article from home page will be

  • /component/content/2-category/20-article

Thus, the generated link depends on the page on which it was generated and on type of home page.

To workaround it, I suppose you will advice to create menu item for every category or for one for "the categories view". OK, it helps. But as we can see, many people did not do it.

If you create a new category without menu item (you forgot) and we do not have a menu item for "the categories view" then link to new article from that category can be:

  1. /archive/5-new-top-category-without-menu/50-article - if we are looking at it from the archive page
  2. /category-2-with-menu/5-new-top-category-without-menu/50-article - if we are looking at it from category with menu item
  3. /article-with_own-menu/5-new-top-category-without-menu/50-article - If we are on article with menu and our module display link to article without own menu item.
  4. /5-new-top-category-without-menu/50-article - we see it from home page or from different component view.
  5. /component/content/article/5-new-top-category-without-menu/50-article - if our home page is login page (login view from com_users) or other component view (not com_content).

My PR stopped generates link like in 1-4 and only generates from point 5.

This way an administrator will see an URL like "/component/..." and can understand that should create a new menu item for that category.

Currently (in 3.8.3) It is difficult to find such a category because link to article may have different links.

The other thing is JRoute::_('index.php?option=com_content'); when a lots of developers use it as "a part of URL" and assume that Itemid will be inherited from active menu item.

The correct code should be JRoute::_('index.php'); which do the same and it is correct.

Every menu item requires option and view so in JRoute::_('index.php?option=com_content'); the view is missing and the full link is incomplete but Joomla allows for that because all URL inherit active menu item in 3.8.3 and lower.

@OctavianC
Copy link
Contributor

If you go directly to page /component/content/article/1-article then that page does not have any Itemid in joomla 3.8.3 with single language. This is a behaviour of all joomla versions.

How would a user end up on such a page if not by a router that's not inheriting the current Itemid?

The correct code should be JRoute::_('index.php'); which do the same and it is correct.

Please don't change 10 years of behavior just to solve a problem that's actually a configuration issue. It really shouldn't matter if I type index.php, index.php?option=com_test or index.php?option=com_test&view=test - the behavior needs to be consistent across all of them.

Every menu item requires option and view so in JRoute::_('index.php?option=com_content'); the view is missing and the full link is incomplete but Joomla allows for that because all URL inherit active menu item in 3.8.3 and lower.

I need to specify a different view & sometimes a task as well, yet I expect the Itemid to be carried over because:
a) I depend on parameters of the menu item
b) I need to redirect back to the parent menu item (eg. task to delete a submission index.php?option=com_test&task=submission.delete&id=1 then redirect to the listing index.php?option=com_test&view=submissions)
c) I need to display subpages eg. index.php?option=com_test&view=submission&id=1
There are more cases, but the above are the ones the original PR broke and I don't want to see that happening again.

@Bakual
Copy link
Contributor

Bakual commented Feb 3, 2018

If you go directly to page /component/content/article/1-article then that page does not have any Itemid in joomla 3.8.3 with single language. This is a behaviour of all joomla versions.

To my knowledge, that URL wouldn't have generated by JRoute like this. It would look rather like this: /component/content/article/1-article?Itemid=101. That happens if the router can't find a matching Itemid for a given component (in this case for com_content). Then it falls back to that component URL, but it will still append an Itemid. Either the one from the homepage, the passed one or the active one. If you end up with an URL without Itemid generated by JRoute, then that is a bug which needs fixing.

@ggppdk
Copy link
Contributor

ggppdk commented Feb 3, 2018

If you end up with an URL without Itemid generated by JRoute, then that is a bug which needs fixing.

there is a an exception to this rule ? a login URL ?

and if last selectable menu item for a login is not accessible by current user (last selectable is the home page menu item), then it is should not be added, right ? because you will get a redirect loop

@Bakual
Copy link
Contributor

Bakual commented Feb 3, 2018

There should be no exception, there should always be an Itemid. Basically Joomla breaks as soon as there is no Itemid.
If the login isn't accessible by the user (eg because the homepage isn't public and there is no login menuitem), then the site owner needs to fix his site setup. It's not a bug in the router.

@Cyrusxxx
Copy link

Cyrusxxx commented Feb 4, 2018

There should be menu ID always. Since Joomla 4 is in development, developers will have enough time to adjust its code for this transition and users will benefit from it.

@csthomas only components affected by this in joomla 3.8.4 are for example kunena, jomsocial and similar ones which didn't have menu ids for their links/menus etc. (except main links) and believe me when I say not having menu ids is simply wrong and when menu id is there we can do so much more as users.

Keep the menu id rule in J4 it will be truly beneficial.

Thank you!
P.S.
I am not as smart as you people I am just a humble user and this is how I see it.

@infograf768
Copy link
Member

If the login isn't accessible by the user (eg because the homepage isn't public and there is no login menuitem), then the site owner needs to fix his site setup. It's not a bug in the router.

This is a specific case which can be solved by not using JRoute for the redirect.
See #19559

@alessio-gaggii
Copy link

There should be menu ID always. Since Joomla 4 is in development, developers will have enough time to adjust its code for this transition and users will benefit from it.

@Cyrusxxx do you really think all developers will update their extensions? This is the same exact case as forcing the Strict Mode in the MySQLi Driver. If all this were to happen, then Joomla 4 would support less than 10% of the currently stable third party extensions for Joomla <= 3.8.3.
I'm not sure why, and for who, you believe changing 10 years of behavior is beneficial.

Keep the menu id rule in J4 it will be truly beneficial.

@Cyrusxxx IMO, there is no CMS that has such a perfect and complete framework like Joomla. However, it looks like third party extensions are no longer that important to Joomla, or at least to some people.
This is going to be a harsh example, but hopefully it will let you see a difference. WP introduced the Shortcode API syntax in v2.8 (May 2008) and 10 years after, they are still talking about backward compatibility with Plugins, by supporting the old syntax.
I believe backward compatibility, when possible, is truly beneficial.

@mbabker
Copy link
Contributor

mbabker commented Feb 4, 2018

This is a specific case which can be solved by not using JRoute for the redirect.

That's wrong too IMO. If we can't rely on the router to build correct URLs, there are bigger issues and bypassing it is just yet another workaround, not a fix.

@Cyrusxxx
Copy link

Cyrusxxx commented Feb 4, 2018

@alessio-gaggii I still think that J4 should go forward with this and I think that developers would update their extensions to be compatible.
If joomla is becoming better with new and better coding why wouldn't developers use new and improved methods for their extensions. But for J3 series I agree compatibility should be there.
But for J4 I think this should be implemented. ;)

@alessio-gaggii - WP introduced the Shortcode API syntax in v2.8 (May 2008)

Yes that is true, but in 2015 for example when WP 4.2.3 was released for some security issues (I cant remember exact details) half of those shortcodes for example were not working after WP update and what happened next 3rd party developers updated plugins to be compatible.
And progress of WP achieved in that moment. ;)

@csthomas
Copy link
Contributor Author

csthomas commented Feb 4, 2018

How would a user end up on such a page if not by a router that's not inheriting the current Itemid?

But joomla has com_tags router which work in that way.

@csthomas
Copy link
Contributor Author

csthomas commented Feb 5, 2018

UPDATED

Because there are a lot of people here who want to go back to URL routing from 3.8.3, I suggest to test (reversion in com_content / archive) #19561 before releasing incoming version 3.8.5.

@csthomas
Copy link
Contributor Author

csthomas commented Feb 5, 2018

@OctavianC

Every menu item requires option and view so in JRoute::_('index.php?option=com_content'); the view is missing and the full link is incomplete but Joomla allows for that because all URL inherit active menu item in 3.8.3 and lower.

I need to specify a different view & sometimes a task as well, yet I expect the Itemid to be carried over because:
a) I depend on parameters of the menu item
b) I need to redirect back to the parent menu item (eg. task to delete a submission index.php?option=com_test&task=submission.delete&id=1 then redirect to the listing index.php?option=com_test&view=submissions)
c) I need to display subpages eg. index.php?option=com_test&view=submission&id=1
There are more cases, but the above are the ones the original PR broke and I don't want to see that happening again.

After the problematic changes was reverted in 3.8.5-rc, I will focus only on J4 in my answers but I may use examples from 3.x.

I understand that you want to use inheritance of Itemid in your generated links and my idea was clear:

  • instead of JRoute::_('index.php?option=com_test&view=submission&...'); please use JRoute::_('index.php?view=submission&...');.
    This way the option and Itemid parameter will be inherited as before.

The main change was,

  • the generated link with parameter option has ceased to inherit the Itemid parameter from the active page.
  • the generated link without parameter option (without Itemid) inherits Itemid as before

If I will create once again my PR for J4 then you only need to remove option=com_test& from JRoute::_('index.php?option=com_test&view=submission&...'); and all will be work as was before.

@OctavianC
Copy link
Contributor

I don't really see the benefit of removing option=com_test from the URL other than for the sake of changing something.

@csthomas
Copy link
Contributor Author

csthomas commented Feb 5, 2018

Then I continue,

com_content has two views (article and archive).

The article view has segments and we can create links like /menu-category-alias/12-category/12-article. The segments are 12-category and 12-article.

The archive view should also be able to use its own segments, like /archive/2018 or /archive/2018/2 where 2018 is a year and 2 is a month.

This two views (artice and archive) should work independently but if the article or its categories does not have any menu item and we are on the archive page, then we get link:

  • /archive/12-category/12-article

but we also would like to use links like:

  • /archive/2018/2
  • /archive/2018

When we go to home page (featured view) then link to the article will be:

  • /1-category/12-category/12-article

The problem starts when the article and its categories does not have any menu item
and the joomla allows to create and parse links like /archive/1-category/12-category/12-article.

Now we have links:

  • /1-category/12-category/12-article
  • /archive/1-category/12-category/12-article
  • /archive/2018

Link /archive/1-category/12-category/12-article is generated because the article does not have own menu item and inherits Itemid from archive view.

The parser of com_content will have a problem because it will try to interpreting 1-category as a year and 12-categoryas a month. Because of that #19561 has been created.

At now the parser uses a workaround and ignore view of menu item /archive and treats it as the article view or the category view.

What do you suppose to get when you are on /archive page and you see link like:

  • /archive/2018

You may be surprised that instead of the list of articles from 2018 you get an article with ID=2018.

The legacy router of com_content try to workaround it and adds an addition query parameter &view=archive to overrides the SEF routing:

  • /archive/2018?view=archive

This is the reason why "views" should not inherit Itemid from each other when we do not want to.

The conclusion is, the article view should not inherit Itemid from the archive view.

I added an option of not inheriting Itemid when link contains option=... in JRoute::('index.php?option=...');

@OctavianC
Copy link
Contributor

How about you implement this in com_content's router.php instead of modifying how the SiteRouter behaves (which affects everyone)?
I still don't see the point in having ALL developers change their code, (with the possibility of breaking something else along the way), just so you can fix an issue with com_content. If it was a B/C change that couldn't be avoided (such as implementing Bootstrap 4 or namespacing) I'd understand, but at this point it just alienates 3rd party developers for no reason at all.

@csthomas
Copy link
Contributor Author

csthomas commented Feb 6, 2018

I can not do this (maybe I do not know how) only for com_content, the problem is general and should be fixed in SiteRouter on J4.

In the J4 version, all component developers will have to rewrite their own (legacy) router.php to new, modern routing. This will be a good time to do that modification.

@joomdonation
Copy link
Contributor

In the J4 version, all component developers will have to rewrite their own (legacy) router.php to new, modern routing. This will be a good time to that modification.

Look like this is not correct. I don't think we have to use modern routing for our extensions. The router class of our extensions just needs to implement build and parse method properly and it should still work well on J4 (Core component com_banners for example, still use old routing code and still work in J4)

@csthomas
Copy link
Contributor Author

csthomas commented Feb 6, 2018

Look like this is not correct. I don't think we have to use modern routing for our extensions.

Yes. I got lost in version 4.

@mbabker
Copy link
Contributor

mbabker commented Feb 6, 2018

The thing that absolutely is not supported in 4.0 is the old function based routers. You must have a class based router implementing the component's RouterInterface and this is done typically by extending either the RouterBase class (which basically just lets you copy/paste the code in your old function based routers into the methods required by that class) or the RouterView class (which is mostly the "modern" routing in core components).

@GiottoVerducci
Copy link

GiottoVerducci commented Feb 6, 2018

My custom component did suffer from this breaking change. :( . Anyway, I applied csthomas's guideline: removing the option parameter as suggested in his example.

The correct code should be JRoute::_('index.php'); which do the same and it is correct.

I notice that JRoute does return the correct value, but when using that value as the form submit value (POST), I get a 303 "See other" code that redirects me towards "/component/customcomponent/?<myparameters>"

Browsing to the url returned by JRoute works fine (GET).

Did anything change concerning re-routing with that (breaking) change?

@csthomas
Copy link
Contributor Author

csthomas commented Feb 6, 2018

@GiottoVerducci Joomla 3.8.5 has been released. Try it.

@mavrosxristoforos
Copy link

As a third-party extension developer since Joomla 1.5, I have strived to develop following the rules. I haven't always achieved that. I am guilty, but sometimes I just didn't know how to do it.

Coding for Joomla is very inconsistent

I feel tired of adding if-elses to my code, just to be compatible with the various Joomla versions. I have clients with professional websites, running 2.5 until today (April 3, 2018). Is that correct? No, it isn't! But what can I do? I have told them many times to update their website, but people just postpone it. So, I am the one who is constantly in the middle.
As OctavianC mentioned correctly, this change is not necessary because of external factors. What is wrong with keeping option=com_test to my links? Why can't the new router just work with that? If it's such a big problem, why can't it just remove it from links generated from within the same component, instead of breaking everything?

Problem

I tried routing a full link with itemid, but it still didn't work if option is there. So, here's the erratic behavior:
https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Router/SiteRouter.php#L342 supposedly tries to get an Itemid from the URI, but https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Router/SiteRouter.php#L219 just sets it to null whatsoever.

Conclusion

Do not set the Itemid to null, if the code is explicitly setting it. That will only cause more trouble.

P.S.: Just for the sake of comparison, Android 4.2 was released in 2012, just like Joomla 2.5. Most new apps still work fine with that version, without much, if any, effort at all.

@joomla-cms-bot joomla-cms-bot changed the title [RFC][4.0] Should routing changes from 3.8.4 remain in 4.0 or should be reverted too? [4.0] [RFC] Should routing changes from 3.8.4 remain in 4.0 or should be reverted too? Aug 21, 2018
@csthomas
Copy link
Contributor Author

I wrote a new PR #22229 for Joomla 4.

In fact, any call to JRoute::_('index.php?option=com_customcomponent') so without &Itemid=n, no longer rewrites the URL correctly, but becomes /component/customcomponent/ instead.

This problem has been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests