Act icon indicating copy to clipboard operation
Act copied to clipboard

HTML title of a news entry should be the news title (not only "News")

Open perlpunk opened this issue 12 years ago • 5 comments

The current HTML title of a news entry is only "News".

perlpunk avatar Feb 13 '13 18:02 perlpunk

Surprisingly, this is not easy with the way the current templates are made: a news item is actually a news list with one item. Same template, different code paths. But the title is set from within the template. Not saying it's impossible, but it would need to be modified to detect how many news items there are, and in the case there's only one, set the page title with the news title. My TT2-fu is currently too weak for this.

maddingue avatar Nov 30 '14 15:11 maddingue

What exactly is the problem?

In the following, the news entries seem to have their title shown properly:

  • http://act.qa-hackathon.org/qa2014/news
  • http://act.qa-hackathon.org/qa2014/news/1166

book avatar Dec 01 '14 09:12 book

Oh, I got it: by "HTML title" you mean the content of <html><head><title>.

book avatar Dec 01 '14 09:12 book

OK, so the main ui template seems to have support for a title variable. This is the one you want to set.

Looking at Act::News, that seems to collect all the news items (from the news_item table) into a hash keyed by language. I think the main title and text methods are actually incorrect. They look like the following (in lib/Act/News.pm):

sub title { $_[0]{title} }
sub text  { $_[0]{text}  }

but in fact, these would be empty/undef, because the actual title and text are under the items hash.

Therefore I would suggest replacing those by:

sub title { $_[0]{items}{$Request{language}}{title} }
sub title { $_[0]{items}{$Request{language}}{text} }

to provide auto-localization. This has the drawback of adding a dependency on $Request{language}, which is probably undesirable. And since those methods are not used anywhere (because there are broken), we might as well just remove them.

The real issue here is to set the title template variable. Individual news items are shown using the news/item template and handled by the Act::Handler::News::Edit handler (which also shows them, it seems).

Template variables are set using this line:

$template->variables( %$fields );

So we need to fill in the news item title, properly localized.

At lib/Act/Handler/News/Edit.pm line 135, we see this:

    if (exists $Request{args}{news_id}) {                                 
        $fields = { %$news };                                             
        $fields->{items} = $news->items;                                  
    }                                                                     

We can simply add the title with this line:

        $fields->{title} = $news->{items}{ $Request{language} }{title};

As an aside, setting $fields->{items} directly is probably not needed, since there is already an items key in $news.

book avatar Dec 01 '14 09:12 book

Title set in 1fadb2c422381626cb940bfaa7714e34e8ceb6e9.

Some cleanup done in 4c629dd80a427b58d5e066857794c82415381ac6.

These haven't been tested.

book avatar Dec 01 '14 09:12 book