Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Backport MAGETWO-57796 and MAGETWO-66885 for Magento 2.1 #10131

Closed
wants to merge 2 commits into from
Closed

Backport MAGETWO-57796 and MAGETWO-66885 for Magento 2.1 #10131

wants to merge 2 commits into from

Conversation

hostep
Copy link
Contributor

@hostep hostep commented Jul 3, 2017 •
edited
Loading

Description

This fixes an issue in the tinymce editor when editing a wysiwyg field on the product edit form pages, where inserted images would get an incorrect url. (not sure if this also fixes the same issue on the category edit form, didn't test this)

I've included MAGETWO-66885 because MAGETWO-57796 introduced a new bug: #9452

See the commit messages to see from which commits I cherry-picked this.

Be aware, some of these issues mention MAGETWO-59690 as the internal ticket number for backporting this to 2.1, if this has already been done, you can close this PR.

Fixed Issues (not 100% sure about all these, and I might have missed some)

  1. inserted image in product description got broken on front end  #6138: inserted image in product description got broken on front end
  2. inserted image in product description got broken on front end  #6088: inserted image in product description got broken on front end
  3. Images in Product description disappear #7306: Images in Product description disappear
  4. Dynamic Media URLs in Products not working properly #7215: Dynamic Media URLs in Products not working properly
  5. images added with WYSIWYG in category description are uploaded into admin-directory #7393: images added with WYSIWYG in category description are uploaded into admin-directory
  6. WYSIWYG is not working and removes images in product #7618: WYSIWYG is not working and removes images in product
  7. Wysiwyg editor image upload not working in magento 2 #9292: Wysiwyg editor image upload not working in magento 2
  8. Magento CE 2.1 Catalog Admin Page cannot parse media url in html content #5444: Magento CE 2.1 Catalog Admin Page cannot parse media url in html content
  9. Using the WYSIWYG removes URL in the description #6851: Using the WYSIWYG removes URL in the description
  10. Backport MAGETWO-57796 and MAGETWO-66885 for Magento 2.1 #10131: Image link broken on product Description and short description part.

(Completely offtopic: is there any way for the community to know what you guys already backported to 2.1? Why aren't you using the 2.1-develop branch for this? It looks like only community contributions go to that branch. It would be highly appreciated if we could see what Magento is backporting themselves, that would help out tremendously.)

shiftedreality and others added 2 commits July 3, 2017 19:25
@shiftedreality @hostep
MAGETWO-57796: [GitHub] Inserted image in product description got bro…
15ec17e 
…ken on front end magento#6138

(cherry picked from commit a657749)

MAGETWO-57796: [GitHub] Inserted image in product description got broken on front end magento#6138

(cherry picked from commit e3d67ea)

MAGETWO-57796: [GitHub] Inserted image in product description got broken on front end magento#6138

(cherry picked from commit 228ee3a)

MAGETWO-57796: [GitHub] Inserted image in product description got broken on front end magento#6138

(cherry picked from commit 9030e08)
@VladimirZaets @hostep
MAGETWO-66885: Special Characters like % in widget inside of WYSIWYG …
3c14085 
…gives an error magento#9452

(cherry picked from commit 19a6485)
@ishakhsuvarov ishakhsuvarov self-assigned this Jul 4, 2017
@ishakhsuvarov ishakhsuvarov added this to the July 2017 milestone Jul 4, 2017
@Bartlomiejsz Bartlomiejsz mentioned this pull request Jul 5, 2017
Closed
@ishakhsuvarov
Copy link
Contributor

Hi @hostep
Unfortunately we can not accept this PR at the moment because issue is already fixed internally and is currently staged for 2.1.8 release.
I can see, that fix was not published to the 2.1-develop branch by Magento development team, therefore not available on the GitHub.
Based on this experience we are currently working out a better branching strategy which would eliminate possibility of this happening again.

Please feel free to comment on the PR or reach us directly in the Community Engineering slack channel if you have any questions.
Thank you for collaboration.

@korostii
Copy link
Contributor

korostii commented Jul 5, 2017

eliminate possibility of this happening again

IMHO keeping the status of corresponding GitHub issues up to date with your internal tasks and branches would help a lot:

  • just setting either a milestone like 2.1.8 or an assignee to the issue, if the backport is being actively worked on, and\or is staged for a certain release internally already, etc,
  • an up for grabs label otherwise

@ishakhsuvarov
Copy link
Contributor

@korostii I agree. We are going to implement one of the solutions asap.

@hostep
Copy link
Contributor Author

hostep commented Jul 5, 2017

@ishakhsuvarov: now that you guys have opened up the 2.1.8-preview branch (which is awesome, thanks for that!), I can see you backported MAGETWO-57796 but not MAGETWO-66885.
Can you make sure you don't forget that, as it contains a very very annoying bug: #9452
This line should be removed: https://github.com/magento/magento2/blob/c5ea1ba/lib/web/mage/adminhtml/wysiwyg/tiny_mce/setup.js#L350

Thanks!

@hostep
Copy link
Contributor Author

hostep commented Jul 6, 2017

@ishakhsuvarov: can you confirm you read my last comment? So this bug doens't make it in the final 2.1.8 version. Otherwise there will be a lot of new issues being opened around this bug after 2.1.8 was released I'm afraid.

//cc @okorshenko

@hostep
Copy link
Contributor Author

hostep commented Jul 6, 2017 •
edited
Loading

Yes, what I'm reporting is that MAGETWO-59690 contains a bug which MAGETWO-66885 fixes, but MAGETWO-66885 hasn't been backported yet, that's what I'm trying to say ;)
See: https://github.com/magento/magento2/blob/c5ea1ba/lib/web/mage/adminhtml/wysiwyg/tiny_mce/setup.js#L350
This line shouldn't exist.

@okorshenko
Copy link
Contributor

Hi @hostep backport ticket for MAGETWO-66885 will be MAGETWO-68810. It is in our backlog. I will inform our POs about this issue one more time. Thank you

@hostep
Copy link
Contributor Author

hostep commented Jul 6, 2017

Ok thanks! :)

@okorshenko
Copy link
Contributor

Hi @hostep
I talked to 2.1.8 release team. They will try to include this fix based on your request. I will be following this issue till 2.1.8
Thank you for pushing this

@ishakhsuvarov
Copy link
Contributor

@hostep Fix should now be available in the 2.1.8-preview branch.

@hostep
Copy link
Contributor Author

hostep commented Jul 10, 2017

Thank @ishakhsuvarov, looks good! :)

@hostep hostep mentioned this pull request Jul 25, 2017
Closed
@Talv Talv mentioned this pull request Nov 22, 2017
Open
@Talv Talv mentioned this pull request Jan 1, 2018
Open
@hostep hostep mentioned this pull request May 11, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ishakhsuvarov ishakhsuvarov

Labels
Release Line: 2.1
Projects
None yet
Milestone
July 2017
Development

Successfully merging this pull request may close these issues.
6 participants
@hostep @ishakhsuvarov @korostii @okorshenko @shiftedreality @VladimirZaets