WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page #25556

engcom-Golf · 2019-11-11T13:55:54Z

Description (*)

Reworked version for #23966

Fixed Issues (if relevant)

Search Adobe Stock button is not active after 2nd attempt to open a media gallery from the same place in Admin adobe-stock-integration#622: Search Adobe Stock button is not active after 2nd attempt to open a media gallery from the same place in Admin
2.WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page #18548 WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page

Manual testing scenarios (*)

[case 1]

Open the catalog product edit page
Open the "content" area"
Enable the WYSIWYG-Mode for the Description
Hide the last editor in the same area (eg. Short Description or ItemDescV)
Click on Insert Image in the Description Editor
Click on the browse-icon for the Image URL
Select a image from the media browser
Click on insert file

[case 2]

Log in to the admin panel
Go to Content > Elements > Pages
Edit any page, such as Home page
Expand Content section
Click on Insert/edit image toolbar button
Click on Browse button of the Source field
Refresh the page and repeat until reproduced

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

…/622

m2-assistant · 2019-11-11T13:55:57Z

Hi @engcom-Golf. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

VladimirZaets

Hi, please take a look at my review comments

VladimirZaets · 2019-11-11T17:48:30Z

lib/web/mage/adminhtml/wysiwyg/tiny_mce/tinymce4Adapter.js

                wUrl = this.config['files_browser_window_url'] +
                    'target_element_id/' + this.getId() + '/' +
                    'store/' + storeId + '/';

+		this.initialize(this.getId(), this.config);


We will call initialize 3 times, it's a very strange approach, why we need to do it?

VladimirZaets · 2019-11-12T17:07:19Z

lib/web/mage/adminhtml/browser.js

            ) {
                if (typeof options.closed !== 'undefined') {
                    this.modal.modal('option', 'closed', options.closed);
                }
                this.modal.modal('openModal');

                return;
+            } else if (typeof options === 'undefined' 


Check to undefined it isn't the best solution. You can check just like if (options) or in case if you really need to check to undefined ( maybe in some cases it's really needed ) we should use _.isUndefined method from underscore library because typeof has some side effects

magento-engcom-team · 2019-11-12T19:10:50Z

Hi @VladimirZaets, thank you for the review.
ENGCOM-6272 has been created to process this Pull Request
✳️ @VladimirZaets, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

magento-engcom-team · 2019-11-12T19:10:52Z

@engcom-Golf thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

VladimirZaets

Please take a look at my review comment

VladimirZaets · 2019-11-13T16:53:14Z

lib/web/mage/adminhtml/browser.js

            ) {
                if (typeof options.closed !== 'undefined') {
                    this.modal.modal('option', 'closed', options.closed);
                }
                this.modal.modal('openModal');

                return;
+            } else if (_.isUndefined(options) && self.modalLoaded === true) {


You can combine inner if with (_.isUndefined(options) && self.modalLoaded === true)

magento-engcom-team · 2019-11-13T23:07:44Z

Hi @VladimirZaets, thank you for the review.
ENGCOM-6272 has been created to process this Pull Request

engcom-Alfa · 2019-11-14T08:37:21Z

✔️ QA Passed

magento-engcom-team · 2019-11-14T11:36:43Z

Hi @sivaschenko, thank you for the review.
ENGCOM-6272 has been created to process this Pull Request

…ly with multiple WYSIWYG-Editors on page #25556

m2-assistant · 2019-11-15T17:44:46Z

Hi @engcom-Golf, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Fix issue with WYSIWYG editors magento/adobe-stock-integration/issues…

f229056

…/622

engcom-Golf requested a review from DrewML as a code owner November 11, 2019 13:55

magento-engcom-team added Area: Lib/Frontend Release Line: 2.3 labels Nov 11, 2019

engcom-Golf requested a review from sivaschenko November 11, 2019 13:56

VladimirZaets requested changes Nov 11, 2019

View reviewed changes

ghost assigned VladimirZaets Nov 11, 2019

ghost added the Progress: needs update label Nov 11, 2019

WYSIWYG Image-Popup is not working correctly with multipleEditors

a07627c

Nazar65 changed the title ~~[WIP] WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page~~ WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page Nov 11, 2019

VladimirZaets reviewed Nov 12, 2019

View reviewed changes

Refactor to use _.isUndefined

8e39f49

VladimirZaets approved these changes Nov 12, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Nov 12, 2019

VladimirZaets added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Nov 12, 2019

engcom-Alfa self-assigned this Nov 13, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Nov 13, 2019

Nazar65 added 2 commits November 13, 2019 14:16

Fic static test, resolve issue by 2 insert image widget

3be5ac8

Fix static test

76a866e

VladimirZaets requested changes Nov 13, 2019

View reviewed changes

ghost added Progress: needs update and removed Progress: testing in progress labels Nov 13, 2019

VladimirZaets approved these changes Nov 13, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Nov 13, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Nov 14, 2019

Refactor per review comment

26dca10

ghost added Progress: extended testing and removed Progress: testing in progress labels Nov 14, 2019

engcom-Foxtrot self-assigned this Nov 14, 2019

ghost added Progress: accept and removed Progress: extended testing labels Nov 14, 2019

sivaschenko approved these changes Nov 14, 2019

View reviewed changes

ghost assigned sivaschenko Nov 14, 2019

This was referenced Nov 15, 2019

FIX Magento Search Please! #622

Closed

WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page #18548

Closed

WYSIWYG image upload dialog hangs up #23966

Closed

magento-engcom-team pushed a commit that referenced this pull request Nov 15, 2019

ENGCOM-6272: WYSIWYG-Editor Insert Image-Popup is not working correct…

261c61d

…ly with multiple WYSIWYG-Editors on page #25556

magento-engcom-team merged commit 26dca10 into magento:2.3-develop Nov 15, 2019

magento-engcom-team added this to the Release: 2.3.5 milestone Nov 15, 2019

sivaschenko mentioned this pull request Nov 18, 2019

Find opportunities to speed up MFTF tests magento/adobe-stock-integration#652

Closed

Nazar65 mentioned this pull request Dec 11, 2019

Cover magento/magento2#25556 with jasmine test #25992

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page #25556

WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page #25556

engcom-Golf commented Nov 11, 2019

m2-assistant bot commented Nov 11, 2019

VladimirZaets left a comment

VladimirZaets Nov 11, 2019

VladimirZaets Nov 12, 2019

Nazar65 Nov 12, 2019

magento-engcom-team commented Nov 12, 2019

magento-engcom-team commented Nov 12, 2019

VladimirZaets left a comment

VladimirZaets Nov 13, 2019

magento-engcom-team commented Nov 13, 2019

engcom-Alfa commented Nov 14, 2019

magento-engcom-team commented Nov 14, 2019

m2-assistant bot commented Nov 15, 2019

WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page #25556

WYSIWYG-Editor Insert Image-Popup is not working correctly with multiple WYSIWYG-Editors on page #25556

Conversation

engcom-Golf commented Nov 11, 2019

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Nov 11, 2019

VladimirZaets left a comment

Choose a reason for hiding this comment

VladimirZaets Nov 11, 2019

Choose a reason for hiding this comment

VladimirZaets Nov 12, 2019

Choose a reason for hiding this comment

Nazar65 Nov 12, 2019

Choose a reason for hiding this comment

magento-engcom-team commented Nov 12, 2019

magento-engcom-team commented Nov 12, 2019

VladimirZaets left a comment

Choose a reason for hiding this comment

VladimirZaets Nov 13, 2019

Choose a reason for hiding this comment

magento-engcom-team commented Nov 13, 2019

engcom-Alfa commented Nov 14, 2019

magento-engcom-team commented Nov 14, 2019

m2-assistant bot commented Nov 15, 2019