Giter Club home page Giter Club logo

Comments (16)

mikehaertl avatar mikehaertl commented on May 24, 2024

Not sure if I understand, because you seem to mix two topics here.

  1. "Root URLs"
    Not sure what you mean by that. / is not a valid route. And if you pass an empty string to createUrl(), the Yii manual states:

If the route is an empty string, the currently requested route will be used;

  1. Redirect for different URLs

What exactly does not work or why do you think URLs with/without a trailing slash incorrect? The redirect URLs keep the trailing slash if there is any.

from yii2-localeurls.

ivan-redooc avatar ivan-redooc commented on May 24, 2024

Yep, sorry, I mixed up too much elements and missed the point.

About the empty route (empty string), your note is relative to yii\helpers\Url::to() as you can see at https://github.com/yiisoft/yii2/blob/master/docs/guide/runtime-routing.md.

While createRoute("") return a "/", if the application is in the document root or is not in a sub-folder.
I hope that using an empty string with createUrl is correct behavior.

The incoherence is related to the default URL or the main/top url of a site (root-url):

  • the URL where the browser is sent to, when arrive the first time on the site by redirectToLanguage, /it
  • the URL where the links point creating the root-url by createUrl(""), that's /it/ (see UrlManager.php:200 )

I hope to be more clear this time

from yii2-localeurls.

mikehaertl avatar mikehaertl commented on May 24, 2024

Hmm, I still can't see, why this should be a problem. If you used a trailing / in your URL, you'll also see a / in the redirect URL. If not, then there's no /. Why do you think this is a problem?

from yii2-localeurls.

ivan-redooc avatar ivan-redooc commented on May 24, 2024

Hi,
what you describe:

If you used a trailing / in your URL, you'll also see a / in the redirect URL. If not, then there's no /

It's not true in case of 'enableDefaultLanguageUrlCode' => true and the URL is http://www.example.org/ because the browser is redirected to http://www.example.org/it .

So I request an URL with trailing / and I got an URL without trailing /

IMHO redirectToLanguage should use createUrl. I'll do a pull request with the my proposal, if you don't mind.

from yii2-localeurls.

mikehaertl avatar mikehaertl commented on May 24, 2024

Please don't - there's good reason, why redirectToLanguage() is the way it is now. We can not use createUrl() because we don't know the route at that point.

So I request an URL with trailing / and I got an URL without trailing /

I'm still not sure, if there's a problem at all. To me this issue is just cosmetic. There's nothing broken - or do you get an error at some point?

from yii2-localeurls.

ivan-redooc avatar ivan-redooc commented on May 24, 2024

No, no error, except I've 2 different URLs (for Google they're) with the same content.
I know I can fix with the canonical-meta but it's not likely.

I undersdand you reasons about redirectToLanguage(), but please give it a chance:

    /**
     * Redirect to the current URL with given language code applied
     *
     * @param string $language for back compatibility.
     */
    protected function redirectToLanguage($language)
    {
        $result = parent::parseRequest($this->_request);
        if ($result !== false) {
            list ($route, $params) = $result;
            $_route = [$route] + $params + $_GET;
        } else {
            throw new \yii\web\NotFoundHttpException(Yii::t('yii', 'Page not found.'));
        }
        $redirectUrl = $this->createUrl($_route);
        Yii::$app->getResponse()->redirect($redirectUrl);
        if (YII_ENV_TEST) {
            throw new \yii\base\Exception(\yii\helpers\Url::to($redirectUrl));
        } else {
            Yii::$app->end();
        }
    }

from yii2-localeurls.

mikehaertl avatar mikehaertl commented on May 24, 2024

I've tested your code - about half of the test cases is failing now. So this does not work.

Again, to make this more clear to me: Can you please describe again, what exactly are the cases where you would expect a different output? Please leave away anything related to createUrl(). This has nothing to do with the issue.

The real question is: Which redirects are not OK in your opinion? So only give me a list of URLs and the resulting redirect URL that you consider wrong.

from yii2-localeurls.

mikehaertl avatar mikehaertl commented on May 24, 2024

And as you mentioned duplicate content: Do you suffer from the same problem as described in #24 maybe?

from yii2-localeurls.

ivan-redooc avatar ivan-redooc commented on May 24, 2024

I write before but I mess-up with markdown :( aplogies.

If the URL I set in the browser is http://www.example.org/
the browser is redirected to http://www.example.org/it
I expected to be http://www.example.org/it/

from yii2-localeurls.

mikehaertl avatar mikehaertl commented on May 24, 2024

Ok. The open question still is: Why do you expect a trailing slash? Even with Yii you never get a trailing slash except for the root URL.

Yii::$app->urlManager->createAbsoluteUrl(['site/index']);
// gives: http://www.example.com/

Yii::$app->urlManager->createAbsoluteUrl(['site/login']);
// gives: http://www.example.com/site/login

from yii2-localeurls.

mikehaertl avatar mikehaertl commented on May 24, 2024

I think I finally understand what your problem is: You are on a page with an URL like http://www.example.com/it/some/page. Now on that page you want to create the root URL:

$url = Url::to(['site/index']);

This now gives you http://www.example.com/it/ with a trailing slash. To be consistent there should not be a trailing slash in this case.

If this is your problem: I've commited a change, that should fix it. Can you help testing it?

from yii2-localeurls.

ivan-redooc avatar ivan-redooc commented on May 24, 2024

It's work fine!
Thank you.

I don't want bother you, and I promise it's last time I push it. This version pass all the tests.

    /**
     * Redirect to the current URL with given language code applied
     *
     * @param string $language the language code to add. Can also be empty to not add any language code.
     */
    protected function redirectToLanguage($language)
    {
        $result = parent::parseRequest($this->_request);
        if ($result === false) {
            throw new \yii\web\NotFoundHttpException(Yii::t('yii', 'Page not found.'));
        }
        list ($route, $params) = $result;
        if($language){
            $params[$this->languageParam]=$language;
        }
        $redirectRoute = [$route] + $params + $_GET;
        $redirectUrl = $this->createUrl($redirectRoute);
        Yii::$app->getResponse()->redirect($redirectUrl);
        if (YII_ENV_TEST) {
            // Response::redirect($url) above will call `Url::to()` internally. So to really
            // test for the same final redirect URL here, we need to call Url::to(), too.
            throw new \yii\base\Exception(\yii\helpers\Url::to($redirectUrl));
        } else {
            Yii::$app->end();
        }
    }

from yii2-localeurls.

mikehaertl avatar mikehaertl commented on May 24, 2024

Hmm, OK, I'm convinced. I probably thought, that parent::parseRequest() can't work because we could have a URL with a language code. But we "fix" this via setPathInfo() in line 232, so we can rely on parseRequest() afterwards. And now that the test cases (hopefully) cover even the most esoteric configurations I think we can consider this change to be safe.

Two things left:

  1. I've replaced your [$route] + $params + $_GET with a simpler array_unshift($params, $route). Why do you think we should also merge the $_GET params? They should be exactly the same as $params already.

  2. Yii has a minor inconsistency when creating a "root URL" for apps that run from a subdirectory, or that have showScriptName set to true. So the URL you get from createUrl(['/']) looks like:

    • /baseurl/
    • /index.php/

    Whereas other URLs like createUrl['site/login'] look like:

    • /baseurl/site/login
    • /index.php/site/login

    Notice the missing trailing slash here. I had to update my test cases, because the extension behaved differently and always created URLs without a trailing slash. But this is now an issue with Yii itself. If you agree, I will not fix that, to be consistent with Yii's default behavior.

from yii2-localeurls.

ivan-redooc avatar ivan-redooc commented on May 24, 2024

Hi

  1. I found it in Request::resolve() and the comment preserve numeric keys induce me to replicate the code:
    public function resolve()
    {
        $result = Yii::$app->getUrlManager()->parseRequest($this);
        if ($result !== false) {
            list ($route, $params) = $result;
            $_GET = $params + $_GET; // preserve numeric keys

            return [$route, $_GET];
        } else {
            throw new NotFoundHttpException(Yii::t('yii', 'Page not found.'));
        }
    }

2 . I agree, it's always better to be consistent.

Thanks for your help
Ivan

from yii2-localeurls.

mikehaertl avatar mikehaertl commented on May 24, 2024

I found it in Request::resolve()

Hmm, ok, then we better also keep it. I'll change it again.

Thanks for your help. Now we finally got rid of some not so nice parts of the code (actually I also never really liked the old implementation - which you can see from the many comments that I added to make it clearer).

from yii2-localeurls.

mikehaertl avatar mikehaertl commented on May 24, 2024

Fixed it and even included the latest change (yiisoft/yii2@df6f270). Changes are now available in latest version 1.2.3.

Thanks again!

from yii2-localeurls.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.