Coroico / AdvSearch

Dynamic content search add-on for MODx Revolution that supports results highlighting, faceted search and search in custom packages
21 stars 14 forks source link

Removed hard coded URI for assets #7

Closed rtripault closed 12 years ago

rtripault commented 12 years ago

Removed hard coded references to assets (JS/CSS) URI

Coroico commented 12 years ago

Romain, your commit doesn't work properly in this case.

if you use base href="[[++site_url]]" in your header and $this->modx->regClientCss($this->modx->getOption('assets_url').'components/advsearch/css/advsearch.css'); in the code.

$this->modx->getOption('assets_url').'components/advsearch/css/advsearch.css' gives /assets/components/advsearch/css/advsearch.css but as [[++site_url]] gives http://www.revo.wangba.fr/ you finally get :

http://www.revo.wangba.fr//assets/components/advsearch/css/advsearch.css with a double / between fr and assets which is not correct!

I just doing a commit with a fix for this issue. Thanks for this feedback.

Coroico commented 12 years ago

Fixed with commit 6d37887

rtripault commented 12 years ago

Coroico,

I'm not sure that's an issue. MODX allows to define a relative path (assets/whatever/path/) or an absolute one (/assets/whatever/path/, http://assets/whatever/path/ or //assets/whatever/path/ should be fine/allowed allowed paths).

For the later, trimming '/' might be an issue… (albeit i'm not sure "//" is allowed in RFCs ^^).

I'll search some more about that this afternoon. Anyway, much thanks sir.

Edit : seems like "//" is valid (see http://www.ietf.org/rfc/rfc1738.txt, " 3.1. Common Internet Scheme Syntax ")