? Success
Pull Request for # 4961

User tests: Successful: Unsuccessful:

avatar GregoryRusakov
GregoryRusakov
3 Mar 2015

Fixed https base tag generation.
Discussion is here:
#4961 (comment)

avatar GregoryRusakov GregoryRusakov - open - 3 Mar 2015
avatar zero-24 zero-24 - change - 3 Mar 2015
Easy No Yes
avatar joomla-cms-bot joomla-cms-bot - change - 3 Mar 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 7 Mar 2015
Category Libraries
avatar zero-24 zero-24 - change - 7 Mar 2015
Title
Update head.php
Fix https base tag generation.
avatar zero-24 zero-24 - change - 7 Mar 2015
Rel_Number 0 4961
Relation Type Pull Request for
avatar joomla-cms-bot joomla-cms-bot - change - 7 Mar 2015
Title
Update head.php
Fix https base tag generation.
avatar Hackwar
Hackwar - comment - 8 Mar 2015

Reading the code, it seems as if the base is set wrong somewhere. If you don't use getBase(), we can't influence this from for example a plugin. So why are you not fixing the place where the base in JDocument is set to a false value and instead replace the call with the static JURI::base()?

avatar nonumber
nonumber - comment - 16 Mar 2015

I think the issue might be in libraries/cms/application/site.php
Which has:

if ($router->getMode() == JROUTER_MODE_SEF)
{
    $document->setBase(htmlspecialchars(JUri::current()));
}

Shouldn't that be?

if ($router->getMode() == JROUTER_MODE_SEF)
{
    $document->setBase(htmlspecialchars(JUri::base()));
}
avatar Sulpher
Sulpher - comment - 1 Sep 2015

Peter provided solution, but it wasn't included in Joomla 3.4.3. Please add this patch in next Joomla version. Thank you.

avatar mbabker
mbabker - comment - 1 Sep 2015

If someone opens a pull request with his suggested change and it gets tested then it can be included in a future release. Until then, there isn't much that can be done. http://magazine.joomla.org/issues/issue-sept-2015/item/2837-did-they-fix-it-yet offers some good insight on how you can help ensure issues that you are having with the core software get resolved :smile:

avatar Sulpher
Sulpher - comment - 1 Sep 2015

Michael, thanks for the link. Sure, it's important to test all patches.
Well, I've tested this patch on various sites with SSL and Varnish - it works. Since the status of this fix is marked as checked I thought it's already revised.

avatar mbabker
mbabker - comment - 1 Sep 2015

The checks block just above the comment box here shows the status of our automated test suites, mainly to make sure that unit tests and codestyle checks continue to pass. Unless a pull request has the RTC label or has already been merged, then it still needs human review and testing.

avatar Sulpher
Sulpher - comment - 1 Sep 2015

Ok, thanks for explanation. How many reviews are usually required to accept the patch?

avatar GregoryRusakov GregoryRusakov - reference | 2825452 - 1 Sep 15
avatar Bakual
Bakual - comment - 2 Sep 2015

Usually two.

avatar b2z
b2z - comment - 12 Sep 2015

@GregoryRusakov can you update your PR to @nonumber solution? Seems that it is the right one.

avatar wojsmol
wojsmol - comment - 17 Sep 2015

@b2z I sent PR #7902 which implements solution suggested by @nonumber. @GregoryRusakov please test mentioned PR.

avatar zero-24 zero-24 - close - 17 Sep 2015
avatar zero-24
zero-24 - comment - 17 Sep 2015

Thanks @wojsmol so I'm closing here in favor of #7902 thanks

avatar zero-24 zero-24 - change - 17 Sep 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-09-17 23:43:51
Closed_By zero-24
avatar zero-24 zero-24 - close - 17 Sep 2015
avatar wojsmol
wojsmol - comment - 25 Sep 2015

@Sulpher please test PR.#7902

Add a Comment

Login with GitHub to post a comment