mikespub-org / seblucas-cops

Calibre OPDS (and HTML) PHP Server : web-based light alternative to Calibre content server / Calibre2OPDS to serve ebooks (epub, mobi, pdf, ...)
http://blog.slucas.fr/en/oss/calibre-opds-php-server
GNU General Public License v2.0
55 stars 7 forks source link

Info: Avoid using relative paths in templates #48

Closed mikespub closed 2 months ago

mikespub commented 9 months ago

You may have noticed templates being updated recently, adding {{=it.whatever}}/ paths for assets, images, resources etc. It's because I've been moving COPS from using relative paths to absolute paths in templates and links - under the optional full_url

The main reasons are:

Sorry for the inconvenience, and try to avoid those relative URLs from now on - thanks ;-)

Note: in templates you can use {{=it.basedir}} if you don't have a more specific variable for the directory already, and in code you can use Route::url() to generate the right URLs.

dunxd commented 9 months ago

Commit [402907aa529914981e50f93f6a1237964431ae70] broke things for me - index.php does not display anything. Viewing source, all the hrefs start with //. If I switch to server side, the page loads, but no images or links work. This is for all the templates.

I did run composer update --no-dev -o and install php-tokenizer after checking out the latest version. Is there something else I need to do? Rolling back to the commit prior to that everything works, so something not quite right with the {{it.images}} etc in templates.

dunxd commented 9 months ago

Looking in the commit I see that the {{it.images}} links were already in the templates, so it must have been something else.

mikespub commented 9 months ago

Hmm - that doesn't sound good. I tried to make it transparent for most installed cases, but I may have missed something.

Do you have anything defined for $config['cops_full_url'] and did you set anything for $config['cops_use_route_urls'] or not ?

And what does your web server + cops setup look like:

dunxd commented 9 months ago

I'm developing in a docker container with vscode. The docker container base is Alpine linux with the following packages added:

sudo apk add --no-cache php php-intl php-sqlite3 php-pdo_sqlite php-gd php-mbstring php-zip php-dom php-openssl php-xmlwriter php-xml php-tokenizer composer

I did add php-tokenizer today as when running composer update --no-dev -o it appeared to be required by something new/updated.

I am using the built in php server, just started with php -S 0.0.0.0:8000. The root directory is whichever directory the cmd is in when invoking the command - I am running it from the folder at the root of the git repo.

No proxy involved - I am just testing locally.

dunxd commented 9 months ago

Just to clarify - Alpine linux installs php 8.1.23 with the above command.

dunxd commented 9 months ago

When viewing source using the commit after I synced my fork the first link is:

<link rel="apple-touch-icon" href="//images/icons/icon57.png">

When I roll back to commit [aaf1da2276eae2ee99ddc2741048a78441e7cf20] the first link is:

<link rel="apple-touch-icon" href="images/icons/icon57.png">

All the other links from the file.html template show the same behaviour.

mikespub commented 9 months ago

Yup - I can reproduce it, thanks. Guess those double // at the start are understood by browsers to point to a domain under the same scheme (https: or http:) instead of gracefully ignoring it like they do further down in the path /something//or/other...

At least I know where to look to fix this now, thanks ;-)

dunxd commented 9 months ago

While rolling back the last three commits on my fork, I noticed that you had done some additonal changes so I synced, and things are working again :-)

mikespub commented 9 months ago

Excellent, thanks

mikespub commented 9 months ago

... I did add php-tokenizer today as when running composer update --no-dev -o it appeared to be required by something new/updated. ...

BTW php-tokenizer should only be needed in development mode.

In production mode:

$ composer update --no-dev -o
$ composer check-platform-reqs
Checking platform requirements for packages in the vendor dir
... no trace of ext-tokenizer here ...
$ composer depends ext-tokenizer -t
There is no installed package depending on "ext-tokenizer"

In development mode:

$ composer update -o
$ composer check-platform-reqs
Checking platform requirements for packages in the vendor dir
...
ext-tokenizer  8.2.10     success
...
$ composer depends ext-tokenizer -t
ext-tokenizer 8.2.10 The tokenizer PHP extension
├──nikic/php-parser v4.17.1 (requires ext-tokenizer *)
│  ├──phpunit/php-code-coverage 9.2.29 (requires nikic/php-parser ^4.15)
│  │  └──phpunit/phpunit 9.6.13 (requires phpunit/php-code-coverage ^9.2.28)
│  │     └──mikespub/seblucas-cops dev-main (requires (for development) phpunit/phpunit ~9.6)
│  ├──sebastian/complexity 2.0.2 (requires nikic/php-parser ^4.7)
│  │  └──phpunit/php-code-coverage 9.2.29 (requires sebastian/complexity ^2.0)
│  │     └──phpunit/phpunit 9.6.13 (requires phpunit/php-code-coverage ^9.2.28)
│  │        └──mikespub/seblucas-cops dev-main (requires (for development) phpunit/phpunit ~9.6)
│  └──sebastian/lines-of-code 1.0.3 (requires nikic/php-parser ^4.6)
│     └──phpunit/php-code-coverage 9.2.29 (requires sebastian/lines-of-code ^1.0.3)
│        └──phpunit/phpunit 9.6.13 (requires phpunit/php-code-coverage ^9.2.28)
│           └──mikespub/seblucas-cops dev-main (requires (for development) phpunit/phpunit ~9.6)
└──theseer/tokenizer 1.2.1 (requires ext-tokenizer *)
   └──phpunit/php-code-coverage 9.2.29 (requires theseer/tokenizer ^1.2.0)
      └──phpunit/phpunit 9.6.13 (requires phpunit/php-code-coverage ^9.2.28)
         └──mikespub/seblucas-cops dev-main (requires (for development) phpunit/phpunit ~9.6)
mikespub commented 2 months ago

Clean-up old information issues