magento / magento2-page-builder

Magento2 PageBuilder
Other
75 stars 57 forks source link

Add YouTube NoCookie Support #827

Closed sprankhub closed 3 months ago

sprankhub commented 1 year ago

Description (*)

Adds support for youtube-nocookie.com domain.

Bug

826

Fixed Issues (if relevant)

  1. magento/magento2-page-builder#826: YouTube Nocookie URL Not Accepted

Builds

Manual testing scenarios (*)

See magento/magento2-page-builder#826.

Questions or comments

Checklist

BorisovskiP commented 1 year ago

Tested and can confirm this patch does fix the issue.One thing to note is that in order for the youtube-no-cookie link to be loaded by pagebuilder, the Maximum Width must be specified. Otherwise the following error is thrown:

Temporarily allowed to save HTML value that contains restricted elements. Allowed HTML tags are: div, a, p, span, em, strong, ul, li, ol, h5, h4, h3, h2, h1, table, tbody, tr, td, th, tfoot, img, hr, figure, button, i, u, br, b, iframe, style, pre, svg, animate, animateMotion, animateTransform, circle, clipPath, defs, desc, ellipse, feBlend, feColorMatrix, feComponentTransfer, feComposite, feConvolveMatrix, feDiffuseLighting, feDisplacementMap, feDistantLight, feFlood, feFuncA, feFuncB, feFuncG, feFuncR, feGaussianBlur, feImage, feMerge, feMergeNode, feMorphology, feOffset, fePointLight, feSpecularLighting, feSpotLight, feTile, feTurbulence, filter, foreignObject, g, image, line, linearGradient, marker, mask, metadata, mpath, path, pattern, polygon, polyline, radialGradient, rect, set, stop, switch, symbol, text, textPath, title, tspan, use, view

sprankhub commented 1 year ago

Hmm indeed, unfortunately, it is a bit more involved than this :-/ I will try to work on a proper fix.

bluemwhitew commented 1 year ago

Another issue you'll most probably face is that Page Builder will revert back to using www.youtube.com when it reads the saved data and pre-populates the Video content type's fields.

For example, both https://www.youtube.com/watch?v=dQw4w9WgXcQ and https://youtu.be/dQw4w9WgXcQ (note the different domains) get converted to https://www.youtube.com/embed/dQw4w9WgXcQ when the data is read, meaning if the content is re-saved - even without changes - it'll go back to using the cookie-based domain.

sprankhub commented 1 year ago

With my latest changes, this should now work as expected. Mind testing, @bluemwhitew?

paras89 commented 1 year ago

@magento run all tests

magento-automated-testing[bot] commented 1 year ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

sprankhub commented 1 year ago

@bluemwhitew, mind testing the changes from this PR?

@paras89, I think the failing tests are unrelated to this PR...?

sprankhub commented 1 year ago

@paras89, would you be able to have a look at this PR?

convenient commented 1 year ago

I mistakenly approved this rather than 👍 , was on autopilot and too used to approving internal Ampersand PRs, whoops 😆

bluemwhitew commented 1 year ago

@sprankhub,

I would highly suggest adding (or extending) Magento Functional Testing Framework (MFTF) tests to support this changeset, which will make it easier to validate.

sprankhub commented 1 year ago

Thanks for the advice, @bluemwhitew. I value my time highly, though. If any Adobe official said "we'll merge this as soon as there is MFTF coverage", I'll consider implementing this. Until then, I save my time for more valuable tasks...

sprankhub commented 1 year ago

@sidolov, I feel bad for tagging you directly, but I currently do not know how else I could move this forward. Would you be able to move this forward?

sprankhub commented 7 months ago

@magento run all tests

magento-automated-testing[bot] commented 7 months ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

sprankhub commented 7 months ago

@magento run all tests

magento-automated-testing[bot] commented 7 months ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

sprankhub commented 7 months ago

@engcom-Hotel, I extended a fitting MFTF test. The failing WebAPI Tests look unrelated to me.

Could you have another look, please?

engcom-Hotel commented 7 months ago

@magento run WebAPI Tests

magento-automated-testing[bot] commented 7 months ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

bluemwhitew commented 7 months ago

Awesome job on this, @sprankhub! 🤘🏻

engcom-Bravo commented 7 months ago

Hi @sprankhub,

Thanks for the collaboration & contribution!

:heavy_check_mark: QA Passed

Preconditions:

Manual testing scenario:

Before: :heavy_multiplication_x: 

Screenshot 2023-11-24 at 2 16 40 PM

After: :heavy_check_mark:  

Screenshot 2023-11-27 at 12 08 34 PM

New-Page-Pages-Elements-Content-Magento-Admin

devarul commented 6 months ago

This PR is in development by an internal team, it will be completed and delivered to the mainline according to our standards and processes. All the original community commits will be preserved.

engcom-Hotel commented 4 months ago

Thanks @devarul for the updates. We are moving this PR On Hold till that time.