The [current-page:title] is broken on some routes

This is still happening in 2.0.x!

Ok, it seems that "hook_metatags_attachments_alter()" isn't working as expected anymore, or the implementation in the "metatag_test_integration" test module needs a fix.

Maybe someone else could take a look? Otherwise, this seems to work as intended! :)

I think we need to expand the test coverage to confirm the problem.

This should be considered with Major priority. I would tend to say: The current-page:title is the most important part in the browser windows tab and search engine preview of a website and so-to-say THE meta tag. Descriptions of the Priority → and Status → values can be found in the Drupal project issues → documentation.

I think we need to expand the test coverage

While I agree with test coverage, I really see a deep impact here in every page presented, if this project is used. Are we sure to not merge it at least on the dev branch for those who are willing to take the risk at their side?

Ok, it seems that "hook_metatags_attachments_alter()" isn't working as expected anymore...

Yeah ... average users may did not even recognize it soon, as long as they do not run some website test calls, because this issue seem to appear silently of a sudden and you start to think: when does this happened and how long is the website in this state? Also common in such cases like this long holding issue is to put a note on the project page linking to this issue here, so that users can chime in and help on test coverage and anything else to be needed.

I would chime in but I would have to go from ground zero into metatag's code base and my time is limited atm because my burn-out from last year did teach me lessons. But if I can make some time free and nothing moved forward here I would surely come back on this. But not before end of this year, I guess.

Hiding outdated patch to focus eyes on @grevil's MR. Also for those who need this and are on dev branch anyway you can test this issue by adding this to the repositories in your composer.json:

  "repositories": [
    {
      "type": "vcs",
      "url": "https://git.drupalcode.org/issue/metatag-2880604.git"
    },
...
  ],

And then run: composer require drupal/metatag:dev-2880604-the-current-pagetitle-is

Oh, that's why. We need to track MR against 2.1.x dev now...

Just had a look into some projects, where we're using
[current-page:title] | [site:name]
as global pattern.

Following the steps and for example visiting /node/add/page I'm not able to reproduce the issue!
The page title is Web-/Landingpage erstellen | My Drupal project as expected.

Also I didn't see any recent reports of this since #21
Maybe it was caused by a conflict with another module or a solved bug?

I can't really remember why @Grevil started working on this, maybe we had the issue somewhere? I don't know, but currently I can't reproduce it.

@dqd could you perhaps try to reproduce it in a vanilla Drupal installation or at simplytest.me using the steps from above?

@dqd could you perhaps try to reproduce it in a vanilla Drupal installation or at simplytest.me using the steps from above?

As the title says "on some routes" ... This issue is not about vanilla Drupal install issues with metatag. We need to keep track on the reports because it is not warrantied that this is fixed. Maybe only hidden in your case.

@anybody: Sorry, I hope you do not feel offended but please do not let us clutter the issue now by one report who cannot reproduce the issue no more in running projects. And this issue is not about vanilla Drupal install but about how things can get into the way so that current-page:title is empty if called from the wrong angle. It is reported why this can happen and that it has its corner cases. And it has been explained why from top to bottom briefly. I pinged @Grevil on Slack an hour ago and he would chime in later if possible, which will possibly be the best bet here to go on before keeping people busy.

@dqd all good, but we need clear steps to reproduce this and I can't see them or reproduce it.

My manual test and feedback regarding node create / edit came from this sentence in the issue summary:

The token [current-page:title] uses the title resolver service which only looks for title in the route definition. If it won't find any value it will return NULL. You can see this in the node edit form where the page title is set within the form.

Maybe that was too simple, yes.

Still I think this definitely needs clear steps to reproduce (manually), in the issue summary, to ensure the tests are correctly based on that. That was my point. We can't even be sure the test here is complete / correct yet. Any things may have changed since 2017.

I edited my comment in the time you answered quickly... So, maybe there is some context confusion now. Again, steps to reproduce are there and testing on vanilla install is meaningless regarding the issue description and the comments following. Latest report about issues on this are from 2021, not from 2017.

Trying to reproduce an issue on a vanille Drupal install which occurs here but not there is like hunting the white rabbit. Again, it would be better and more time saving if people who worked on the latest fixes can have a quick look if the way the token gets fetched is still the issue. If this is not possible for other reasons, no problem, I can have a briefly chat with Damien later if time is there.

Wow THAT! was quick @Grevil +1 Awesome!

I will call out for user tests/reviews. If the issue disappears for them with this MR, then it obviously is still worth merging. +1 @Grevil

Changed the target branch and rebased the fork on current 2.1.x!

@dqd Not really in the picture anymore, with this issue. As @Anybody already mentioned, we are not using the current MR as a patch in any of our projects, but I am very certain the described issue is still a thing, as I am seeing it in our dev environment from time to time.

So feel free to test the MR & adjust the tests to get this fixed ASAP! Thanks for continuing here! 😁👍

I will call out for user tests/reviews. If the issue disappears for them with this MR, then it obviously is still worth merging.

Great stuff! Glad my past self could help! 🎉

Thanks @grevil and @dqd. Thinking about this for a while, we might have had the issues when we worked on https://www.drupal.org/project/homebox → 3.x branch. I think that might have been the place where we ran into this and I asked @Grevil to have a look. Next time, we should also add more context, where we ran into it and which steps are needed for reproduction. :)

FWIW the test failure is a false negative.

Too bad it doesn't seem possible to do a tests-only change anymore using merge requests, without creating another MR with just the tests changes.

The attached patch solves this with one drawback - it regenerates all tokens and ignores the static cache of previously generated metatag data.

That's two drawbacks ;-)

The first drawback is performance-related. Might the second issue might cause problems for people with complicated setups?

It might be worth modifying this to not assume $variables['head_title']['title'] exists.

Also, could it be reworked to only run metatag_get_tags_from_route() if the title existed?

Lastly, the comment on metatag_handle_real_page_title_token() is a bit redundant.

FWIW the test failure is a false negative.

Yeah, that's what I guessed ...

Might the second issue might cause problems for people with complicated setups?

Well any even minor issue cause problems for people with complicated setups. :) Just kidding. TBH, if these concerns only belong on "save" when editing Metatag setup and not on each page load even if all caches are enabled, I think this is tolerable. Setting up MetaTag isn't a minor setup.

Also, could it be reworked to only run metatag_get_tags_from_route() if the title existed?

I can try to have a look when I get a free minute and some sleep.

But I can at least report from 2 folks I spoke with and told them to please tryout the latest MR on Drupal 11 projects, that the page title finally came back on their projects after multiple cache clears and some reloads. Yay.

WAIT! I have to find out if they have updated from 2.1.x to this MR or from 2.0.x dev from project page to this MR. Because 2.1.x is not listed on the project page and other things on 2.1.x could have fixed parts of it.

fwiw the only change in 2.1.x is migration-related.

THX! good to know, so it doesn't matter actually. Still no response yet from the other side, but I will test myself next days...

To come back to the point of regeneration:

The attached patch solves this with one drawback - it regenerates all tokens and ignores the static cache of previously generated metatag data.

... That's two drawbacks: 1st performance-related. 2nd it might cause problems for people with complicated setups?

Well, if I am not mistaken, the regeneration is only happening on save of metatag editing, right? And it could be wanted and maybe untangles for some use cases of this issue that the title token won't be loaded.

Apart from that, Damien, good news: 4 x RTBC from here! Including our self + other users/projects report using it without flaws. Most coming from latest 2.0.2 release or from the Drupal 11 compatibility issue as repo. So we actually can commit it to 2.1.x-dev and 2.0.x-dev

Additionally I tried to resolve/address most parts from #40

It might be worth modifying this to not assume $variables['head_title']['title'] exists. Also, could it be reworked to only run metatag_get_tags_from_route() if the title existed? Lastly, the comment on metatag_handle_real_page_title_token() is a bit redundant.

MR is green now. Some review would be helpful to move on.

I applied merge request 104 to my site and it resolved the issue of not having HTML titles anymore on views pages after I installed metatag.

Though it looks like Needs tests is applied and the merge request may be out-of-date with any recent test fixes so I'm going to update the status to Needs work.