The fix is pretty obvious.

Indeed. Ouch

How could this fail?

@okonomiyaki3000

How could this fail?

not related to this PR. There are only some deprecated Messsages like PHPUnit_Framework_Assert::assertTag is deprecated

This is not currently possible, it's a hidden feature. Do it anyway.

how we can do it if it is currently not possible?

It's only not possible. It's not impossible. Where there's a will, there's a way.

It's only not possible. It's not impossible. Where there's a will, there's a way.

Hehe Bug or Feature?

$params = JComponentHelper::getParams('com_content');
// Allways false / 0 as we have no enty in
// https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/config.xml
$advanced = $params->get('sef_advanced_link', 0);

To test

• Add

$advanced = 1;

to line 39 and 293 in this file: https://github.com/joomla/joomla-cms/blob/staging/components/com_content/router.php

• Access a Article Category Blog e.g. on testing data.
• click on the First Blog Post
• 404 error
• apply patch
• error is gone.

good on @okonomiyaki3000

This comment was created with the J!Tracker Application at http://issues.joomla.org/.

Then the next thing would be to actually add this option to the Content component configs so people can use it. I don't know if there's a reason for keeping it hidden. Is this feature not supposed to be available until some specific future release? I'm for getting new features out asap.

I guess it was once an idea to be added but was buggy as hell and not reliable at that time.
If it now works reliably, I don't see a reason to not make it available. It's something people are asking for quite often.

This PR has to do with the routing and @Hackwar is preparing some code which might render the SEF Advanced Routing useless. So Hannes can you shed some light here?

PS. The double quote is wrong, my comment is about revealing the SEF Advanced Routing option

Yes, the advanced routing will be obsolete as soon as I get my contribution working. Working on it.

It was a reliably feature and worked good, but I forgot to commit the config.xml into Subversion at that time when I implemented it. In the meantime it actually has become buggy because the routers were rewritten and broken numerous times.

So your contribution will also result in urls without article ids in them? Because that's all I really care about.

Well that code is no where to be seen at this time (months late) and we
have something now

On 9 September 2014 15:19, dgt41 notifications@github.com wrote:

This PR has to do with the routing and @Hackwar
https://github.com/Hackwar is preparing some code which might render
the SEF Advanced Routing useless. So Hannes can you shed some light here?

PS. The double quote is wrong, my comment is about revealing the SEF
Advanced Routing option

—
Reply to this email directly or view it on GitHub
#4246 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

There's an idea. If the new way will result in alias-based urls as well then it seems like we could start using the existing advanced links code now (assuming no other bugs) and switch to the new code later if does things in a better/faster/smarter way. To the typical user, the change might be completely transparent.

@okonomiyaki3000 The problem with Advanced SEF, if I am not wrong here, is that may end up really meshy. E.g.: you have a category with an article alias test route is /cat-name/test
You write another article down the road, you put a title test and there you have it. Two articles resolve in the same url.
My 2 cents here:
There is a solution, we can see how K2 is handling the save item (there is a check on the alias and if exists just adds a number on the end test1).
With this check on the alias i can’t imagine a faulty scenario.
Edit
Also Save to new and Batch need some logic so they don’t spill out the same alias

@okonomiyaki3000 and here is the code that can be put on a helper as a check alias function or similar

        // Check if alias already exists. If so warn the user
        $params = JComponentHelper::getParams('com_k2');
        if ($params->get('k2Sef') && !$params->get('k2SefInsertItemId'))
        {
            $db = JFactory::getDBO();
            $db->setQuery("SELECT id FROM #__k2_items WHERE alias = ".$db->quote($this->alias)." AND id != ".(int)$this->id);
            $result = count($db->loadObjectList());
            if ($result > 1)
            {
                $this->alias .= '-'.(int)$result + 1;
                $application = JFactory::getApplication();
                $application->enqueueMessage(JText::_('K2_WARNING_DUPLICATE_TITLE_ALIAS_DETECTED'), 'notice');
            }
        }

I hope Fotis won’t mind if we use this code

We actually already have that type of validation in Joomla core, see JModelAdmin::generateNewTitle()

The model save functions by default use this to check for unique aliases at the levels required.

Thanks @mbabker for sharing this.
So if Joomla create unique aliases there is no way that this can go wrong.
Go for it @okonomiyaki3000 and reveal the option in com_config…
Can we have this one on 3.4?

EDIT:
@mbabker: Although this function might be called in batch and save to new it’s not the case in creating a new article.
I just checked creating another article in the same category with the same title, the result is an error msg, not automatically saving the alias with -1 at the end. So still on creating new article we have to automatically create an incremental alias.

Can we simply add something like this in administrator/components/com_content/models/article.php @ line 465:

        if ($app->input->get('task') == 'apply' && (int) $app->input->get('id') == 0)
        {
            if ($data['alias'] == NULL)
            {
                $data['alias'] = JFilterOutput::stringURLSafe($data['title']);
            }
                list($title, $alias) = $this->generateNewTitle($data['catid'], $data['alias'], $data['title']);
                $data['alias'] = $alias;
        }

Also these lines https://github.com/joomla/joomla-cms/blob/staging/components/com_content/router.php#L315-L323
return 404 if url is domain.ltd/menu-name/some-fancy-category-name or domain.ltd/some-fancy-category-name
inserting this if() restores functionality

            if (!$advanced)
            {
                // We check to see if an alias is given.  If not, we assume it is an article
                if (strpos($segments[0], ':') === false)
                {
                    $vars['view'] = 'article';
                    $vars['id'] = (int) $segments[0];

                    return $vars;
                }
            }

When creating an article with the same alias as another article in the same category, I think that the correct action is to throw an error, not increment the alias. The user should be made aware that the settings he has provided will not work so he can make his own decision about what and how to change things.

When creating an article with the same alias as another article in the same category, I think that the correct action is to throw an error, not increment the alias. The user should be made aware that the settings he has provided will not work so he can make his own decision about what and how to change things.

If you want to change that behavior, please do in another PR

Isn't it the current behavior?
Edit: Yes it is.

My personal opinion:
average Joe don't know the significance of alias in joomla.
He, most of the times, is not filling the field alias.
Throwing an error confuses people, default option should be automagically shave an incremental alias and throw a warning, so if the user DO understand it and WANT to make the appropriate change he will have been informed.
@okonomiyaki3000 here is some confusion that comes from that error statement:
http://forum.joomla.org/viewtopic.php?f=624&t=602058&start=30

Two points here:
If we are going to remove the id from the URL I guess we have to triple check that alias is unique.
In comparing terms alias is what permanent id is is to wp and in wp the user doesn't get to have the option to interfere with that. Joomla I will say should also do the same: handle the alias automagicaly ( no field for average joe) and reveal the filed only to advanced users!
By as @Bakual said let's keep this PR on router
Also my other comment should state we
need to add this if (!$advanced) statement
Not remove that

Isn't it the current behavior?

Both ways are actually used. When using the batch methods, the alias is adjusted automatically. When saving an article, an existing alias will generate a failure.

If we are going to remove the id from the URL I guess we have to triple check that alias is unique.

The uniqueness isn't a problem we have to triple check. That is already enforced as of today.
But you may want to prevent the user from changing the alias in a published item. Because it would break existing links if advanced SEF is enabled. With current SEF, only the id is relevant and the alias is just flavor text.

@Bakual Make the field alias disabled for all except admin might work, in combination to auto filling the alias in new articles, i guess

Automatic increment for new items, specially when unicode alias is on, would be an issue as people may want ascii there for some items

For new items, you can have it enabled without issues. Also for unpublished ones.
It's only an issue if you change the alias after the item has been published.

So I would probably link it to the current state, not to an ACL. Making it readonly if $item->state > 0.

Also it's only an issue if advanced SEF is enabled.

@infograf768 We can have few changes to address this
1. make field alias disabled for published items as @Bakual suggests
2. Auto filling the field alias
3. In Advanced SEF ALWAYS use transliterate so we get english alias which is easy to handle
This is a new feature and actually we can do all these

@dgt41
@Bakual said the opposite: make it enabled...

Why force transliterate in Advanced? I totally disagree with this. It is the admin choice to use Unicode alias or transliterate.

Side Note: let's not forget that we may have menu items with same alias if language is different ( a feature we introduced in 2.5 beta)

@okonomiyaki3000 @Bakual @infograf768 and everybody participating in this:
If we are about to introduce this can we, pretty please, make sure that people don’t accidentally break their sites and hurt again the reputation of the cms?
Alias is gonna be the one and only point of truth, lets protect it (by disabling for most the access to it) by any means.
The same alias under different lang should be ok, not sure I am gonna check it now and comment back.
EDIT:
No save is allowed for the same alias in different languages

lets protect it (by disabling for most the access to it) by any means.

I sincerely hope you will not be followed on this.

For the record, I don't care if this option is publicly enabled or not. I'll write a plugin to enable it myself but I can't even do that as long as this bug exists. That's why this PR addresses exactly one line of code.

If it does get enabled, I'm pretty much on the same page as @infograf768 on all these issues.

@okonomiyaki3000 Can you also address this with a commit

            if (!$advanced)
            {
                // We check to see if an alias is given.  If not, we assume it is an article
                if (strpos($segments[0], ':') === false)
                {
                    $vars['view'] = 'article';
                    $vars['id'] = (int) $segments[0];

                    return $vars;
                }
            }

in https://github.com/joomla/joomla-cms/blob/staging/components/com_content/router.php#L315-L323
This is also a bug for Advanced SEF

@okonomiyaki3000
Agree. We should commit this,
If further improvements are done, then they anyway need a new PR and discussion.

I'll try to have a look at it in the near future but I won't put it in this PR.

@okonomiyaki3000
I was speaking of the original PR

No save is allowed for the same alias in different languages

That is in ROOT, as the message says

here it is

@infograf768 You are right you can save it in another parent, but that qualifies fine in my setup

@okonomiyaki3000 Just to clarify is the buggy code here https://github.com/joomla/joomla-cms/blob/staging/components/com_content/router.php#L315-L323 going to be addressed in this PR or should I make a new PR?

Not this PR. You can make one if you like. If not, I'll make one after I can confirm the problem. I'm not going to try to make sense of it on my phone.

I don’t want to split the problem in many PR’s, so I’ll wait till you read the code and make the adjustments here. To reproduce the problem:
Create a category
Create an article in this category
Create a menu to this article
navigate to this article
click on the category link
a nice 404 error!
apply the code I provide [if (!$advanced) {…}] problem fixed

I can try tomorrow probably. That's Japan time so a little over 12 hours from now or so.

Setting this to RTC from code review. It's a no-brainer and I will merge this evening if nobody else is faster

Similar fixes for the same issue in newsfeed & weblinks routers. PR's #4265 and #4266

@dgt41 I can confirm some kind of problem but it's a little deeper than you think. This router first takes every segment and replaces the first - with : under the assumption that the segment format is <id>-<alias> but that is a wrong assumption in the first place. Let me see what can be done about it.

In any case, when using using advanced links, there is a possibility that a category contains a subcategory and an article which both have the same alias and this causes some ambiguity. A similar thing can occur without advanced links. If you have a category which contains a subcategory and an article which both have the same id, then the category will be used if the alias part of the segment is match with that of the category, otherwise the article is used. If the article and category both have the same id and alias then the category will be used. I guess this works well enough. A similar logic could be used for advanced links I suppose.

I've spent the day looking at this router (the parse function really) and I'm convinced that it's pretty f'ed up. I can surely fix it but I think it requires some discussion. Is the best way to just open a new PR or take this up with the Google group?

OK, so I opened this: #4267
Please discuss. @dgt41 I think it will solve your issue but it also cleans up a lot of other stuff.