admin/url/custom cause media directives to break

AndreKlang commented 6 years ago

Summary

If admin use a custom domain, different from the default website, the wysiwyg image-selector will return malformed media directives, causing broken links to images.

Expected directive: {{media url="wysiwyg/image.jpg"}} Returned directive: {{media url="http://backend.dev/media/wysiwyg/image.jpg"}}

This started when this was done: https://github.com/magento/magento2/issues/12147

Preconditions

PHP 7.1
Magento Open Source 2.2.4
No sample data
One (1) website, store & store_view
web/unsecure/base_url = "http://frontend.dev/" (Default Config)
web/secure/base_url = "http://frontend.dev/" (Default Config)
cms/wysiwyg/use_static_urls_in_catalog = 0
Go to Admin -> Stores -> Configuration -> Advanced -> Admin -> Admin Base URL set Use Custom Admin URL to Yes, Set Custom Admin URL to "http://backend.dev/"

Note: Both frontend.dev & backend.dev can be substituted by anything, but they both have to point to the same installation, and they HAVE to be different from each other.

Steps to reproduce

Go to Admin -> Content -> Pages -> (edit any page)
In the Content wysiwyg editor, click the "Insert Image"-button
Click the browse icon
Select an image (upload one first if you don't have one)
Click insert

Expected result

Preview should show the selected image
The "___directive" part of "Image URL" should base64_decode to {{media url="wysiwyg/image.jpg"}}
Once the image is inserted in page-content and page is saved, img-src on the front end should be "http://frontend.dev/media/wysiwyg/image.jpg"

Actual result

Preview shows a "Skin Image"-Placeholder
The "___directive" part of "Image URL" does base64_decode to {{media url="http://backend.dev/media/wysiwyg/image.jpg"}}
Once the image is inserted in page-content and page is saved, img-src on the front end is "http://frontend.dev/media/http://backend.dev/media/wysiwyg/image.jpg"

Additional information

Reverting https://github.com/magento/magento2/commit/94fdca7b7a46eaf032c8314dc81428ba8aec1126 does fix this issue, but I feel that the issue originates from the method Magento\Cms\Helper\Wysiwyg\Images::getImageHtmlDeclaration().

Specifically these lines, if $fileurl and $mediaUrl have different domains (witch they do in my case), the str_replace won't remove the domain-part. And I can't honestly tell if that is the intended purpose or "never supposed to happen"..

Edit: Given that the frontend url was used in the thumbnails (observed here), I'd guess that there is a deeper issue regarding media-url's and custom admin-url.

magento-engcom-team commented 6 years ago

Hi @AndreKlang. Thank you for your report. To help us process this issue please make sure that you provided the following information:

[ ] Summary of the issue
[ ] Information on your environment
[ ] Steps to reproduce
[ ] Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento-engcom-team give me {$VERSION} instance

where {$VERSION} is version tags (starting from 2.2.0+) or develop branches (2.2-develop +). For more details, please, review the Magento Contributor Assistant documentation.

@AndreKlang do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?

[ ] yes
[ ] no

AndreKlang commented 6 years ago

@magento-engcom-team give me 2.2.4 instance

magento-engcom-team commented 6 years ago

Hi @AndreKlang. Thank you for your request. I'm working on Magento 2.2.4 instance for you

magento-engcom-team commented 6 years ago

Hi @AndreKlang, here is your Magento instance. Admin access: https://i-16427-2-2-4.engcom.dev.magento.com/admin Login: admin Password: 123123q Instance will be terminated in up to 3 hours.

AndreKlang commented 6 years ago

@magento-engcom-team I confirm that I was able to reproduce the issue on vanilla Magento instance following steps to reproduce. However, in somewhat reverse order since I only had access to one working domain. So I first set Admin URL to "https://i-16427-2-2-4.engcom.dev.magento.com/" and then set base_url to "https://i-16427-2-2-4-frontend.engcom.dev.magento.com/".

Also, I noticed that the thumbnails on the image browser returned 404 (since the frontend-domain was used there), and the Preview image returned 500 (There has been an error processing your request)

ghost commented 6 years ago

@AndreKlang, thank you for your report. This issue is already fixed in Magento 2.3.

AndreKlang commented 6 years ago

@engcom-backlog-pb Can you please reference in what issue/commit/pull-request this was solved?

ghost commented 6 years ago

@AndreKlang, unfortunately, no. WYSIWYG editor was totally reworked in Magento 2.3.

dimaportenko commented 6 years ago

Any workarounds until Magento 2.3 release?

dimaportenko commented 6 years ago

I've described my workaround on the StackExchange https://magento.stackexchange.com/a/238101/28975

chris-pook commented 6 years ago

Thanks for the workaround suggestion @troublediehard , this is a pretty serious issue as clients are unable to manage their content without a major upgrade.

chris-pook commented 6 years ago

As a suggestion, should this workaround be an adminhtml after plugin rather than a preference and string_replace the URL out of the returned value entirely (as the core code function does)?

/**
     * Resolve issue with custom admin URL incompatibility with WYSIWYG admin CMS widget.
     * @see https://github.com/magento/magento2/issues/16427
     *
     * @param Images $subject
     * @param $result
     * @return string
     */
    public function afterGetImageHtmlDeclaration(
        Images $subject,
        $result
    ) {

        $adminUrl = $this->scopeConfig->getValue('admin/url/custom');

        return str_replace($adminUrl, '', $result);
    }

dimaportenko commented 6 years ago

@chris-pook there are things I don't like about both solutions, but since it is a workaround I think it's fine.

magento-engcom-team commented 5 years ago

Hi @engcom-backlog-nazar. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if your want to validate it one more time, please, go though the following instruction:

[ ] 1. Add/Edit Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
[ ] 2. Verify that the issue is reproducible on 2.3-develop branch
Details
- Add the comment @magento-engcom-team give me 2.3-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 3. Verify that the issue is reproducible on 2.2-develop branch.
Details
- Add the comment @magento-engcom-team give me 2.2-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.2-develop branch, please add the label Reproduced on 2.2.x
[ ] 4. If the issue is not relevant or is not reproducible any more, feel free to close it.

ghost commented 5 years ago

Hi @AndreKlang The issue was re-tested and we can confirm that it was fixed on the 2.3 release branch. We closing this issue as fixed due to upcoming 2.3 release that will be available soon.

magento / magento2