It may be worth discussing a bit with @Hackwar as I'm sure he did part of this work already in his project.

Works well except of that one case. I am not so sure I explain well enough the steps to reproduce this

I found another case where Advanced will fall back to create a link with id
On a fresh install
Install com_overload and create some dummy content
create a menu in the main menu pointing in a single article

Navigate there

Use the pagination for next or previous

Article id and category id are back!

One more error about the same case test in the parseTradisional() @ line 392 onwards
$cat_id = 0 won’t cut it

The url is http://localhost/tested/overload-1/overload-1-1/overload-1-1-10
tested = menu
overload-1 = cat
overload-1-1 = cat
overload-1-1-10 is article
Note that the id’s are missing

@dgt41 If you're using traditional links, why should the ids be missing? If your site is configured to use traditional links then http://localhost/tested/overload-1/overload-1-1/overload-1-1-10 is not a valid url anyway. The parseTraditional function will never work if the url is malformed like that.

@dgt41 I'm afraid I don't understand your first case either.
So you have Category A and Article B (which is not necessarily in A).
example.com/cat is created by a category type menu item configured for A
example.com/cat/art is created by a single article type menu item configured for B

What's the problem? It seems to work for me. Is this not the setup you're talking about?

Maybe this is the case in multilingual? Oh try not to get a menu for the category that article B belongs

Maybe this is the case in multilingual?

Maybe. I don't have time to test that right now.

Oh try not to get a menu for the category that article B belongs

Seems to be fine whether B is a member of A or not.

@dgt41 I just looked more closely and it seems the trouble is (keep with the earlier example) you would have a 'single article' type menu item at example.com/cat/art but you are attempting to load the page example.com/cat/art/uncategorized and this (correctly!) gives a 404.

Please explain why you would expect anything other than 404 in this case.

@okonomiyaki3000 I cannot spare more time for testing, right now, but although the produced url is obviously wrong, clicking on a category link in the header of an article should somehow resolve and not bring a 404 page. I guess the error is not here, but, maybe, in the way joomla creates those links?

Hmm maybe. I didn't touch the build function at all. It might have some problems just as parse did.

I think we need a more systematic way to test this. At least a list of specific points to check. I can't really see how any of the problems listed above could actually happen and I'd be interested to know if anybody could reproduce them.

@okonomiyaki3000 Tested on a new install and works fine

It sounds like you probably had some other stuff installed that doesn't play well with this feature. If such things exist, they need to be updated but their existence shouldn't hold Joomla back from adding new features. Advanced Links exists now as a kind of secret option. Maybe it can become a public option or maybe it can even be somehow marked as 'experimental' but, until then, I doubt 3rd party developers will spend the time to make their extensions work with it (although I doubt there's really much extra work required).

@okonomiyaki3000: how and what is supposed to be tested here? I understand URLs but what are the steps to reproduce the issue/enhancement.

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

@roland-d this is really about SEF Advanced Links. You need to turn that option on first and you will see that it is basically broken. This patch is meant to fix it.

I'm sure you will want to know how to turn on SEF Advanced Links.

Also, read this thread, especially my line notes, and it should be clear what is being fixed.

@roland-d So, do you get what this is about now or shall I go into some more detail?

Why did Travis fail here? There's no reason for it to fail.

Launched Travis again. At the time the PR was made, we had an issue with Travis and assertTag

Thanks!

@okonomiyaki3000 It seems clear, I haven't had time to check it yet but will get to it over the weekend.

@test: So I enabled the sef_advanced_link to 1 in the router after applying the patch and tested the front end. I don't see any differences before and after the patch with the sample data installed. With the patch applied, everything continue to work as expected.

I have not tested multi-lingual though.

@Hackwar do you have anything to add here?

@okonomiyaki3000 I've done some PRs lately that have implications on your PR. You might want to update your PR with the latest staging. Besides that, all the core components are basically identical and thus have identical routers. If you fix something in one router, you should fix it in the other routers, too.

To be honest, my plan at the moment is to copy the existing routers into a nasty subclass for legacy purposes and not fix anything in them and instead write completely new routers. Everything that we "fix" in the old component routers will in some way change existing URLs and we will have enough B/C issues with this on our hands anyway. So my plan was to have people be able to either select the legacy router and keep the issues that they had so far or to select the new router with the downside that the system might not entirely behave like before. That means that it wont have the strange behavior in edge cases like right now. The target is still 95% URL compatibility, but beginning with that point, with a sane code base below its surface.

@Hackwar

The target is still 95% URL compatibility

This sounds like you're not thinking of dropping ids from urls in favor of aliases.

No, that sounds like I want to give people the option of either having the old URLs and keeping their search engine ranking or to break all existing URLs and use the new ID-less URLs. If you followed the discussions on the mailinglist and in the PRs, you have seen that in order for the new router to be accepted by the project, it needs to be 100% backwards compatible to the existing solution and no URLs are allowed to change.

Well, obviously you'd want an option where nothing changes at all but that's 100% url compatibility. Then, if the 'advanced' option is that only aliases are used and ids are dropped, to me that's probably quite less than 95%.

At the risk of generalizing, it seems to me that, given the option, anyone putting up a new site would choose 'advanced' links but almost nobody with an existing site would. But what if there was a fallback so that, when the 'advanced' parser tries to parse an old link but finds nothing, it falls back on the old one and then, if that works, it gives a 301 redirect to the correct new url. Maybe this is what you're doing, I haven't looked.

Anyway, I'm happy to close this PR if there's another solution already. I only opened it because I came across some bad code and bad code should be fixed.

There will be 4 options:

• Legacy-extreme router: This one is the respective current component router and is 100% backwards compatible. It generates the same crappy URLs as now and it also accepts every crap that you throw at its feet.
• ID-based router: This one generates mostly the same URLs as the current router, but it will only accept and build URLs that are correct and that adhere to the principle of a filesystem structure. That means that you can not reach an article when that article is not in the category that has been parsed prior to that. That means that /menu-item1/23-category-1/42-article1 and /menu-item2/666-category2/42-article1 will not display the same article, but one of those two will display a 404. It also means that the URL will behave like a path in a filesystem. If you have /menu-item1/23-category-1/666-category-2/42-article1 and remove 42-article1, you will get to the category "category-2" and if you further cut 666-category-2 from the URL, you will get to "category-1". This is not the current behavior. The current behavior for a URL like /menu-item1/666-category-1/category-2/42-article1 is to still display category "category-2" if you only have /menu-item1/666-category-1.
• Read-old-URLs router: This one can be enabled in addition to "ID-based router" and allows to also parse some of the old URLs and redirect correctly to the new ones.
• ID-less router: This one will remove the IDs from the URLs.

I want to provide the "legacy extreme" router, because the current routers are so absolutely broken that I don't think that anybody is going to be able to write a clean implementation of those routers that really works in all corner cases. You have to assume that somewhere out there, someone is using some extremely broken URLs and still wants those to work and quite frankly, I think your PR already changes the router so much that there might be cases where such URLs would be broken.

I think your PR already changes the router so much that there might be cases where such URLs would be broken.

I doubt it. I've separated legacy routing into a different function and not touched it at all. The only changes I've made happen if advanced routing is turned on. I have not provided a way to turn it on. So this PR exists only for the purpose of supposedly fixing code that never gets used anyway. Your solution sounds much more complete but I hope that people will not be overwhelmed by having a number of options that seem very similar to one another.

Is it that you can not use the 'Read-old-URLs' router in combination with the 'ID-less' router? Is there some technical reason why that's not possible?

You will be able to use the ID-less router together with the Read-old-URLs router, but that would mean that you can get false positives for some URLs. Is /year-in-pictures/2014 an ID-less router URL or should the legacy router parse this? In that case we would get problems.

Shouldn't it work such that the ID-less router attempts to parse it first and if it doesn't come up with anything only then the Read-old-URLs router would be used? Then any false positives you'd get would be exactly the same as those you might get now. Or maybe the situation is more complex than that...

I have a couple of questions, sorry if this not the right place to ask, I realize they may have been answered someplace else already.
1) What version should we expect these new features to appear in?
2) Will these 4 options always be available or do you plan to deprecate one or two of them eventually?

Anyway, as I said above, I'm interested in ID-less urls so as long as those become a reality, I'm satisfied.

The complete new router should be in 3.5 and the legacy-extreme router should be deprecated for 4.0.

Withdrawing this request since this feature is coming soon anyway.