veda-consulting-company / uk.co.vedaconsulting.mosaico

Other
39 stars 76 forks source link

Regression: Incorrect template path set hence unable to update template #543

Open pradpnayak opened 2 years ago

pradpnayak commented 2 years ago

May be from https://github.com/veda-consulting-company/uk.co.vedaconsulting.mosaico/pull/529

Domain name: https://eample.domain.org/portal-login/

Template saved in db: {"template":"portal-login\/wp-content\/uploads\/civicrm\/ext\/uk.co.vedaconsulting.mosaico\/packages\/mosaico\/templates\/tedc15\/template-tedc15.html","name":"No name","created":1666609008462,"editorversion":"0.15.0","changed":1666609017335}

Later rendered as : https://eample.domain.org/portal-login/portal-login/wp-content/uploads/civicrm/ext/uk.co.vedaconsulting.mosaico/packages/mosaico/templates/tedc15/template-tedc15.html

MtnPavlas commented 2 years ago

Has there been any movement on this please? On Joomla, we're seeing a similar issue where the [civicrm.files] variable gets interpreted by Mosaico as example.com/administrator/media/civicrm whereas it should be example.com/media/civicrm.

Therefore, when Mosaico is trying to render the template, it displays a gray screen with a 404 error in the console, and the template it's trying to pull is, e.g.: example.com/administrator/media/civicrm/ext/uk.co.vedaconsulting.mosaico/packages/mosaico/templates/versafix-1/template-versafix-1.html ... notice the "administrator" directory in the path that shouldn't be there.

Just for kicks, if I generate the entire directory structure and template under the administrator subdirectory (as in the path above), it works for rendering the template, but not for rendering the actual mailings as in that case it throws the following error: Access to XMLHttpRequest at 'https://example.com/administrator/media/civicrm/ext/uk.co.vedaconsulting.mosaico/packages/mosaico/templates/versafix-1/template-versafix-1.html' from origin 'https://example.com' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. Long-story-short, the [civicrm.files] variable needs to be interpreted correctly by Mosaico. Also, hardcoding the Files directory in the CiviCRM core config (in Admin >> System Settings >> Directories ... and Resource URL's), i.e. using the full path vs. the [civicrm.files] variable, does NOT solve this issue. Mosaico still stubbornly inserts "/administrator" in all the paths it tries to access for the templates, uploaded images, etc.

I'm not sure if the root cause is the same as what the OP posted, but if not, I'm happy to open a new issue - please let me know, and thank you so much for looking into this.

jorich-2000 commented 1 year ago

I'm getting the same issue (Joomla! 3.10.10 Stable, CiviCRM 5.54.0) - any news on fixes please I did get around it by adding [civicrm.files]/../mosaico_tpl to the Directory and URL settings

adixon commented 1 year ago

That's a curious initial report. I had a similar experience with a multilingual site, where the /en got duplicated, maybe that's a useful clue, i.e. it was duplicated due to some kind of path prefix relative to the base domain that the code assumed (erroneously)?

timinaust commented 1 year ago

Hi, I too am seeing this problem in Wordpress - with wordpress installed in a subdirectory. I get the subdirectory twice.

My wordpress is installed to siteURL like https://example.com/demo/

In api/v3/MosaicoTemplate.php, the 'create' (lines 33-36) saves the full url path (ie it includes the installation subdirectory) ,

/demo/wp-content/uploads/civicrm/ext/uk.co.vedaconsulting.mosaico/packages/mosaico/templates/versafix-1/template-versafix-1.html

Then when it retrieves it in the 'get' (line 81), it appends that relative path to the siteURL, which includes the subdirectory.

https://example.com/demo/demo/wp-content/uploads/civicrm/ext/uk.co.vedaconsulting.mosaico/packages/mosaico/templates/versafix-1/template-versafix-1.html

Why?

I think in most of the cases here, the preg_match in getDomainFor function (line 189) is finding "template-versafix-1.html" at the end of the template path and wrongly adjusting the siteURL in the get.

The preg_match is matching a domain name like structure at the end of the string. I don't know much about multisite layout, but I suspect this would be OK for host, but not for path, where filename and domain name can look the same.

I would think that when it finds host, you could just return that, and looking for a 'domain name' somewhere near the start of the path (as per Drupal multisite layout), you may want something like.

/(?>\/[^\/]*){1,4}\/(?P<domain>(?>[a-z0-9][a-z0-9\-]{1,63}\.)+[a-z\.]{2,6})\//i

Hope this helps.

TomCrawshaw commented 1 year ago

We are also experiencing this, on a BackdropCMS site installed in a subdirectory (https://weyarun.org.uk/backdrop_demo for example). On trying to open a template for editing, the browser console reports that it is trying to get: GET https://weyarun.org.uk/backdrop_demo//backdrop_demo/files/civicrm/ex... Note the double "backdrop_demo" and "//". We regularly copy our production site to demo, and it looks as if this copies the mosaico metadata in the template database table incorrectly as the metadata is an absolute address. timinaust's idea of changing the preg_match was interesting, but why is this needed? Curiously, I do appear to be able to take a previous mosaico mailing, copy it and edit it for a new mailing, but nothing to do with templates works (including replacing the template in the old mailing). Help!

timinaust commented 1 year ago

In terms of ... 'but why is it needed?'. I don't know. I can understand that in a multisite environment having a full URL with hostname will cause CORS cross-site issues when loading files using javascript, but I would have expected that simply removing the protocol/hostname from the template path, and allowing the browser to do what it normally does would be the simplest and most obvious solution.

This do this, would need to reverse these changes and simply have the return at line 90 in civicrm_api3_mosaico_template_get return only the path component of the stored template.

Having said that, it may be that people are using web server rewrites rules in different multisite environment and these are not working effectively. This was obviously implemented to fix a problem someone was having and there seems to have been a follow-up update to this after the original concept was implemented to fix one use-case regression, but there are potentially different use-cases that are not being considered here, including multilingual ones documented in other issues.

Tim

lcdservices commented 1 year ago

Just noting that this issue persists in Joomla for Mosaico versions 3.0-3.2

lcdservices commented 1 year ago

The issue seems to be with the URL construction logic in civicrm_api3_mosaico_template_get(). In Joomla, the issue is that the baseURL() is returning the backend/admin URL where it should be returning the frontend URL.

I think a general improvement would be for the extension to use Civi::resources()->getUrl('uk.co.vedaconsulting.mosaico'); to retrieve the base extension URL and then build the path to the actual template file from there. But currently the template path is stored relative to the site root, so it would take some work to restructure that.

Joomstack commented 8 months ago

Hi Is there any solution on Joomla 4 and CIVI latest version the issue still exist when edit the template it adding /administrator that result 404

twomice commented 1 month ago

Re: path wrongly inclues /administrator/

I'm seeing this now under Joomla (with equivalent symptoms: gray-screen-of-death in editor; 404 error in console), after upgrading the extension from 2.5.1597918155 to 3.6.1725615009.

Not seeing much activity on this and similar issues in the queue, I "fixed" this by simply adding a symlink so the 'wrong' path would resolve.