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

All pages have title "LinkAce" #691

Closed
cweiske opened this issue Sep 15, 2023 · 2 comments · Fixed by #673
Closed

All pages have title "LinkAce" #691

cweiske opened this issue Sep 15, 2023 · 2 comments · Fixed by #673
Labels
Bug Something isn't working

Comments

@cweiske
Copy link

cweiske commented Sep 15, 2023

Bug Description

My browser history is useless now because all pages in LinkAce have the title "LinkAce".

How to reproduce

  1. Open LinkAce. Page title: LinkAce
  2. Go to "All Links". Page title: LinkAce
  3. Open Settings. Page title: LinkAce
  4. Open System settings. Page title: LinkAce

Expected behavior

Each page should have a sensible page title.

Logs

No response

Screenshots

No response

LinkAce version

v1.12.2

Setup Method

PHP

Operating System

Linux (Ubuntu, CentOS,...)

Client details

No response

@cweiske cweiske added the Bug Something isn't working label Sep 15, 2023
@chrissawyerfan4
Copy link
Contributor

chrissawyerfan4 commented Oct 4, 2023

Related: #584 "Use tag & lists name for HTML title"

Possible solution:

In resources/views/partials/header.blade.php, change the line containing <title> to:

<title>@if(isset($pageTitle)){{$pageTitle}} - @endif{{ systemsettings('system_page_title') ?: config('app.name', 'LinkAce') }}</title>

The separator is chosen as a hyphen, but maybe that should be something else (em dash, pipe) or be configurable. I first made it part of the page title system setting, but then any pages without title (e.g. the homepage) would have the separator prepended as well. That's okay if pages always have titles, though.

The contents of $pageTitle are printed with HTML encoding, so this is not an XSS vector.

Now, the page title can be chosen by the controller, e.g. app/Providers/FortifyServiceProvider.php:

Fortify::loginView(function () {
    return view('auth.login', [
        'pageTitle' => 'Login',
    ]);
});

As another example, here's what I used for tags (app/Http/Controllers/Models/TagController.php):

...
        return view('models.tags.index', [
            'pageTitle' => 'Tags',
            'tags' => $tags,
...
    public function create(): View
    {
        return view('models.tags.create', [
            'pageTitle' => 'Add tag',
        ]);
    }
...
        return view('models.tags.show', [
            'pageTitle' => $tag->name,
            'tag' => $tag,
...
    public function edit(Tag $tag): View
    {
        return view('models.tags.edit', [
            'pageTitle' => 'Edit tag: ' . $tag->name,
            'tag' => $tag,
        ]);
    }

This is how it comes out in my browser:

screenshot of browser title bar and URL bar, showing a title of "games - LinkAce — Mozilla Firefox" with a URL showing that this is tag ID 12

To support translations, it looks like the solution is to add the translation entry and call trans() in the controller for strings such as 'Edit tag:' and 'Add tag'. (I neglected that extra step for my local version.)

Another solution uses views instead of passing it from the controller. I should first have looked up what the standard Laravel way of doing this is instead of just starting with the first idea that came to mind, to be honest. I'd probably have gone for the view-based option.

@Kovah let me know if you would like this as a pull request anyway (with translations support).

@Kovah
Copy link
Owner

Kovah commented Oct 4, 2023

Looks good to me.
By the way @isset($pageTitle){{$pageTitle}} - @endisset should also work.

chrissawyerfan4 added a commit to chrissawyerfan4/LinkAce that referenced this issue Oct 15, 2023
@Kovah Kovah linked a pull request Nov 1, 2023 that will close this issue
@Kovah Kovah closed this as completed in #673 Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants