J4 Issue ?
avatar crimle
crimle
25 Feb 2021

Steps to reproduce the issue

  • Download Joomla 4.0.0-beta7-dev as a zip file
  • unpack zip file locally
  • upload files to webserver using FTP
  • start installation by using any webbrowser
  • at step 5 «Congratulations! Your Joomla! site is ready.» choose «Install additional languages»
  • select «German DE»
  • in the next step select «German DE» as default language (2x)
  • click [Set default language]
  • now click the red button [Remove "installation" folder]

Expected result

The folder [installation] should have been removed.

Actual result

The folder [installation] is still there.

System information (as much as possible)

Joomla 4.0.0-beta7-dev
PHP 8.0.1
MySQLi 10.2.33-MariaDB-log

Additional comments

This happens when you choose an additional language towards the end of installation.
If at step 5 you do not choose an additional language but immediately click [Remove "installation" folder] the folder [installation] will b removed all right.

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
4.00

avatar crimle crimle - open - 25 Feb 2021
avatar joomla-cms-bot joomla-cms-bot - labeled - 25 Feb 2021
avatar infograf768
infograf768 - comment - 25 Feb 2021

Can't confirm this on 4.0-dev as of today.

avatar crimle
crimle - comment - 25 Feb 2021

There is an error in the tag «Build: 4.0.0-beta8-dev». I was using Joomla 4.0.0-beta7-dev downloaded today from https://github.com/joomla/joomla-cms/releases/tag/4.0.0-beta7. I did three test-installations with this package:

  1. choosing an additional language towards the end of installation.
  2. NOT choosing an additional language towards the end of installation.
  3. choosing an additional language towards the end of installation.

Installation 1 and 3 the folder "installation" was not deleted.

avatar Quy
Quy - comment - 25 Feb 2021

@crimle Please test using the Joomla nightly builds: https://developer.joomla.org/nightly-builds.html

avatar crimle
crimle - comment - 25 Feb 2021

I was testing again with the nightly build right now.
Despite my clicking the red button [Remove "installation" folder] the folder was NOT deleted.
Please note: the error only occurs, when choosing an additional language towards the end of the installation.
See attached screenshot.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32516.

avatar chmst
chmst - comment - 25 Feb 2021

Confirmed on a fresh 4.0 dev.
Install a laguage and SET IT AS DEFAULT. Then the installation folder remains after delete button.

avatar chmst chmst - change - 25 Feb 2021
Labels Added: J4 Issue ?
avatar chmst chmst - labeled - 25 Feb 2021
avatar chmst chmst - labeled - 25 Feb 2021
avatar infograf768
infograf768 - comment - 25 Feb 2021

@chmst
I did that setting French as default for admin and site.
I had my test site folder open on the desktop and saw the installation folder being deleted.
My test was done locally.

avatar infograf768
infograf768 - comment - 25 Feb 2021

Please open Console to see any js error

avatar dgrammatiko
dgrammatiko - comment - 25 Feb 2021

The delete is not working on a dev env, it's not a bug

avatar chmst chmst - change - 25 Feb 2021
Priority Urgent Medium
avatar chmst chmst - change - 25 Feb 2021
Labels Removed: ?
avatar chmst chmst - unlabeled - 25 Feb 2021
avatar chmst
chmst - comment - 25 Feb 2021

Delete is working if no language is selected or the language is not set as default. I tested several times on win 10, xampp, php 7.4.
I performed the tests in a local environment, always the same directory

avatar chmst
chmst - comment - 25 Feb 2021

So either it is a bug that the folder ist deleted as @infograf768 and I can see, or it is strange that the folder remains in some situations as @crimle and I have seen ... as long as we are in development is makes no difference.

avatar brianteeman
brianteeman - comment - 25 Feb 2021

@dgrammatiko the difference between a dev and release is that you do not have to delete the installation folder. You should still be able to its just not required

avatar infograf768
infograf768 - comment - 25 Feb 2021

The point here is not that it does not have to be deleted on the dev.
The OP says the button does not work. My tests here show that it works in the conditions stated (install language, make it as default).
The event here is present and the js is working as should.
Maybe a permissions issue? Not sure as I can’t reproduce.

avatar infograf768
infograf768 - comment - 26 Feb 2021

MAMP Macintosh Firefox PHP 7.4.2
Here are some screenshots.

Screen Shot 2021-02-26 at 08 20 48

deletefolder

Console result

Screen Shot 2021-02-26 at 08 23 09

avatar crimle
crimle - comment - 26 Feb 2021

@infograf768
Did you just select «German (Germany)» using the radio buttons?
Or did you click the [Set default language] button afterwards?
And after that the [Remove "installation" folder?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32516.
avatar infograf768
infograf768 - comment - 26 Feb 2021

Oops.. I did not use the set default language button

testing again

avatar infograf768
infograf768 - comment - 26 Feb 2021

I confirm the bug and console does not show any error...

avatar infograf768 infograf768 - change - 26 Feb 2021
Labels Added: ?
avatar infograf768 infograf768 - labeled - 26 Feb 2021
avatar infograf768
infograf768 - comment - 26 Feb 2021

@dgrammatiko @Fedik
Looks like the issue is in the remove.js

avatar Fedik
Fedik - comment - 26 Feb 2021

I not tested, but js just showing the result. And because the response is 200 it thinks that all is okay.

The bug may be due this TODO:

$this->app->getSession()->destroy();
$r = new \stdClass;
$r->view = 'remove';
/**
* TODO: We can't send a response this way because our installation classes no longer
* exist. We probably need to hardcode a json response here
*
* $this->sendJsonResponse($r);
*/

If there any error happen during InstallationController::delete() it should return error response, but it just stays silent.

avatar Fedik
Fedik - comment - 26 Feb 2021

Additionally, this section totaly ignored (because that TODO):

// If an error was encountered return an error.
if (!$success)
{
$this->app->enqueueMessage(Text::sprintf('INSTL_COMPLETE_ERROR_FOLDER_DELETE', 'installation'), 'warning');
}

avatar infograf768
infograf768 - comment - 26 Feb 2021

Note

The Remove Installation folder button only exists if we are in development...

<?php if ($this->development) : ?>
<div  class="alert flex-column mb-1"  id="removeInstallationTab">
<span  class="mb-1 font-weight-bold"><?php  echo Text_('INSTL_SITE_DEVMODE_LABEL'); ?></span>
<button  class="btn btn-danger mb-1"  id="removeInstallationFolder"><?php  echo Textsprintf('INSTL_COMPLETE_REMOVE_FOLDER', 'installation'); ?></button>
</div>
<?php endif; ?>

If I set the Version param to stable, then it does not show at all.

Then if I test without installing extra languages and just click Complete and Open (site or admin), the installation folder is not deleted at all and we do not get to site or admin
we have a real mess here

avatar agi-code
agi-code - comment - 28 Feb 2021

I just checked a page that I had installed with Joomla Beta 7 online. I added a second language here, which is the default language. The installation folder was not deleted. I did not check this at the time of installation.

Just now I tested this again locally. If you select another language and set it as default, then the installation folder is not deleted. There is also no hint or error displayed. You click the delete button and assume that the directory is deleted. You can continue to navigate to site or admin normally.

I'm sure there are a few installations that are online and their webmasters don't read this issue. What happens with this? Is there any way to ensure that the directory is deleted in a future update?

avatar dgrammatiko
dgrammatiko - comment - 28 Feb 2021

Then if I test without installing extra languages and just click Complete and Open (site or admin), the installation folder is not deleted at all and we do not get to site or admin
we have a real mess here

I'll bet that the session is different on the last page than the one on the previous page. I thought this was fixed...

EDIT: can someone test the following: add

Joomla.replaceTokens(successresponse.token);

after

const successresponse = JSON.parse(response);

avatar infograf768
infograf768 - comment - 1 Mar 2021

@dgrammatiko
Does not work here.

There are also other problems to solve when this one is done.

avatar dgrammatiko
dgrammatiko - comment - 1 Mar 2021

Does not work here.

Yes it needs also passing the token from the PHP side $r->token = $app->getFormToken(); in both set and setdefault functions on installation/src/Controller/LanguageController.php

avatar infograf768
infograf768 - comment - 1 Mar 2021

@dgrammatiko
Alas this does not work here.

avatar Fedik
Fedik - comment - 1 Mar 2021

@dgrammatiko is right, it a session issue, server response for task=installation.removeFolder:

Screenshot_2021-03-01_20-07-42

Plus javascript issue because it should show that there an error to User

avatar infograf768
infograf768 - comment - 2 Mar 2021

@dgrammatiko

Please provide patch as a priority. This is an important Release Blocker.
Other notables errors:
The Remove Installation Folder button is only displayed when we are in -dev version

The Important Alert
INSTL_COMPLETE_REMOVE_INSTALLATION="PLEASE REMEMBER TO COMPLETELY REMOVE THE INSTALLATION FOLDER.<br />You will not be able to proceed beyond this point until the &quot;%s&quot; folder has been removed. This is a security feature of Joomla!"
is absent from 4.0.

IMHO, we need to modify the remove/default.php to

					<div class="alert flex-column mb-1" id="removeInstallationTab">
						<?php if ($this->development) : ?>
							<span class="mb-1 font-weight-bold"><?php echo Text::sprintf('INSTL_SITE_DEVMODE_LABEL', 'installation'); ?></span>
						<?php else : ?>
							<span class="mb-1 font-weight-bold"><?php echo Text::sprintf('INSTL_COMPLETE_REMOVE_INSTALLATION', 'installation'); ?></span>
						<?php endif; ?>
						<button class="btn btn-danger mb-1" id="removeInstallationFolder"><?php echo Text::sprintf('INSTL_COMPLETE_REMOVE_FOLDER', 'installation'); ?></button>
					</div>

and modify strings to something like
INSTL_SITE_DEVMODE_LABEL="We detected development mode.<br>The \"%s\" folder should be deleted if you are not testing on localhost."
INSTL_COMPLETE_REMOVE_INSTALLATION="PLEASE REMEMBER TO COMPLETELY REMOVE THE INSTALLATION FOLDER.<br>You will not be able to proceed beyond this point until the \"%s\" folder has been removed. This is a security feature of Joomla!"

We have also to make sure that, after setting Default Languages by using the Set default language button, the final page is no more including the choice to install new languages and setting Default again as we would get in another token issue.

avatar dgrammatiko
dgrammatiko - comment - 2 Mar 2021

@infograf768 not before finishing the Child templates UI...

avatar joomdonation
joomdonation - comment - 5 Mar 2021

Look like the solution is:

@dgrammatiko @Fedik Do we have a javascript API to get the token from the hidden input, something like token = Joomla.getToken(); ? I tested by using the code

if (form) {
	var data = Joomla.serialiseForm(form);
}
else 
{
     var	data = {};
}

then pass that data in ajax request https://github.com/joomla/joomla-cms/blob/4.0-dev/installation/template/js/remove.js#L39 and it works. But I know it is not nice to use that code.

avatar Fedik
Fedik - comment - 5 Mar 2021

Do we have a javascript API to get the token from the hidden input

nope, a new token should come from ajax response I think,
something like in comment @dgrammatiko #32516 (comment)

avatar Fedik
Fedik - comment - 5 Mar 2021

but confusing part is why it changes after set default language,
I cannot find a reason

avatar dgrammatiko
dgrammatiko - comment - 5 Mar 2021

There is an api Joomla.refreshToken() iirc which takes it from the hidden input

avatar Fedik
Fedik - comment - 5 Mar 2021

that something new 😄

avatar dgrammatiko
dgrammatiko - comment - 5 Mar 2021

It’s a useless piece of code that used only in the installation but still bloating core.js. I mean it’s not the only useless thing in that file 😀

avatar dgrammatiko
dgrammatiko - comment - 5 Mar 2021

@joomdonation adding the token should be way easier in this case:

// Get the token from the previous response and save it as `token`
Joomla.request({
	method: "POST",
	// Then just append it here
	url: Joomla.installationBaseUrl + '?task=installation.removeFolder&format=json&' + token + '=1',
	perform: true,
	// token: true,
	headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
	onSuccess: function () {
		const customInstallation = document.getElementById('customInstallation');
		customInstallation.parentNode.removeChild(customInstallation);
		const removeInstallationTab = document.getElementById('removeInstallationTab');
		removeInstallationTab.parentNode.removeChild(removeInstallationTab);
	},
	onError: function (xhr) {
Joomla.renderMessages({ error: [xhr] }, '#system-message-container');
	}
	}
);
avatar joomdonation
joomdonation - comment - 6 Mar 2021

@dgrammatiko I know adding the token is easy. I was asking to see if we have an easy way to get the token from previous response instead of having to:

  • Serialize the whole form inputs and pass it in ajax request like this:
var form = document.getElementById('languagesForm');
			
if (form) {
    var data = Joomla.serialiseForm(form);
}
else
{
   var data = {};
}
  • Or will we have find the token manually, similar with with Joomla.replaceTokens:
Joomla.replaceTokens = function (newToken) {
    if (!/^[0-9A-F]{32}$/i.test(newToken)) {
      return;
    }

    var elements = [].slice.call(document.getElementsByTagName('input'));
    elements.forEach(function (element) {
      if (element.type === 'hidden' && element.value === '1' && element.name.length === 32) {
        element.name = newToken;
      }
    });
  };
avatar joomdonation
joomdonation - comment - 6 Mar 2021

but confusing part is why it changes after set default language,
I cannot find a reason

@Fedik As I understand, for each ajax request, we generate a new token https://github.com/joomla/joomla-cms/blob/4.0-dev/installation/src/Response/JsonResponse.php#L35 , that's why the old token is not valid anymore.

avatar Fedik
Fedik - comment - 6 Mar 2021

hm, I see now, that explain everything,
so as @dgrammatiko said, this section should update/store the token:

onSuccess(response) {
const successresponse = JSON.parse(response);
if (successresponse.messages) {
Joomla.renderMessages(successresponse.messages, '#system-message-container');
}
},

The token also should be updated in ScriptOptions registry:

Joomla.loadOptions({'csrf.token': successresponse.token})

because Joomla.request() pick it from there for POST request:

if (newOptions.method !== 'GET') {
const token = Joomla.getOptions('csrf.token', '');
if (token) {
xhr.setRequestHeader('X-CSRF-Token', token);
}

Then this basically should be done after every request, if we update session token every time.

avatar joomdonation
joomdonation - comment - 6 Mar 2021

Joomla.loadOptions is what I'm looking for :).

avatar richard67
richard67 - comment - 6 Mar 2021

@dgrammatiko @joomdonation @Fedik It is a pleasure to me to see how you are working together. These are the times when I love to be here. We are community, free, from all over the world. Thanks a lot for that.

avatar infograf768
infograf768 - comment - 6 Mar 2021

Can't wait to test a working PR ;)

avatar dgrammatiko
dgrammatiko - comment - 6 Mar 2021

@joomdonation to recap here:

  • in php you need to add $r->token = $app->getFormToken(); in both set and setdefault functions on installation/src/Controller/LanguageController.php
  • Adjust the set language js
    Joomla.request({
    method: 'POST',
    url: `${Joomla.installationBaseUrl}?view=setup&frontendlang=${frontendlang}&administratorlang=${administratorlang}&task=language.setdefault&format=json`,
    perform: true,
    token: true,
    headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
    onSuccess(response) {
    const successresponse = JSON.parse(response);
    if (successresponse.messages) {
    Joomla.renderMessages(successresponse.messages, '#system-message-container');
    }
    },
    onError(xhr) {
    Joomla.renderMessages({ error: [xhr] }, '#system-message-container');
    },
    });
    :
function updateToken(token) {
  let options = Joomla.optionStorage;
 options['X-CSRF-Token'] = token;
  Joomla.loadOptions(options)
}

      Joomla.request({
        method: 'POST',
        url: `${Joomla.installationBaseUrl}?view=setup&frontendlang=${frontendlang}&administratorlang=${administratorlang}&task=language.setdefault&format=json`,
        perform: true,
        token: true,
        headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
        onSuccess(response) {
          const successresponse = JSON.parse(response);
          if (successresponse.messages) {
            Joomla.renderMessages(successresponse.messages, '#system-message-container');
updateToken(successresponse.token) 
          }
        },
        onError(xhr) {
          Joomla.renderMessages({ error: [xhr] }, '#system-message-container');
const successresponse = JSON.parse(response); // This might need a try/catch
updateToken(successresponse.token) 
        },
      });

That should be all 4-5 lines of code needed here

EDIT Oops the Joomla object needs to be invoked here

avatar joomdonation
joomdonation - comment - 6 Mar 2021

@dgrammatiko It would be simpler than that if Joomla.loadOptions works as @Fedik mentioned. I will give it a try later today.

avatar dgrammatiko
dgrammatiko - comment - 6 Mar 2021

@joomdonation actually what I posted was wrong, the token needs to be updated using Joomla.loadOptions

avatar joomdonation
joomdonation - comment - 6 Mar 2021

@dgrammatiko Yes, Joomla.loadOptions is the way to go. Tested and it worked.

avatar joomdonation
joomdonation - comment - 6 Mar 2021

OK. I made PR #32599 which should fix this issue. Maybe more changes will be needed, but the PR should fix this reported issue for now.

avatar richard67 richard67 - change - 6 Mar 2021
Status New Closed
Closed_Date 0000-00-00 00:00:00 2021-03-06 12:43:03
Closed_By richard67
avatar richard67 richard67 - close - 6 Mar 2021
avatar richard67
richard67 - comment - 6 Mar 2021

Closing as having a pull request. Please test #32599 . Thanks in advance.

avatar wilsonge wilsonge - change - 12 Mar 2021
Labels Removed: ?
avatar wilsonge wilsonge - unlabeled - 12 Mar 2021

Add a Comment

Login with GitHub to post a comment