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.

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

amp-iframe, amp-instagram, amp-youtube, amp-soundcloud wrapped in unnecessary p tags #4450

Open
jerclarke opened this issue Mar 26, 2020 · 1 comment · May be fixed by #7564
Open

amp-iframe, amp-instagram, amp-youtube, amp-soundcloud wrapped in unnecessary p tags #4450

jerclarke opened this issue Mar 26, 2020 · 1 comment · May be fixed by #7564
Assignees
Labels
Embeds P2 Low priority Punted WS:Core Work stream for Plugin core
Projects

Comments

@jerclarke
Copy link

jerclarke commented Mar 26, 2020

Bug Description

This is a continuation of #4358 where the same problem applied to amp-facebook and was fixed.

The premise is that these amp-* objects should be in the root of the document, and not be wrapped in <p> tags. Twitter already worked this way, and in #4358 amp-facebook was fixed to match.

I've now noticed that amp-iframe, amp-instagram, amp-youtube, amp-soundcloud face the same problem:

Screen Shot 2020-03-25 at 7 19 09 PM

Screen Shot 2020-03-25 at 7 21 08 PM

Screen Shot 2020-03-26 at 3 50 05 PM

As with the previous ticket, it's important to me because I need to style something that immediately follows the object (amp-instagram+.translation), but also generally it just seems like good practice all around.

Instagram embed code:

[embed]https://www.instagram.com/p/Buq8vU6Hcxn/[/embed]
<blockquote class="translation">This is a translation of the Instagram post above.</blockquote>

YouTube embed code

[embed]https://www.youtube.com/watch?v=vXPJVwwEmiM[/embed]
<blockquote class="translation">This is a translation of the YouTube video above.</blockquote>

Soundcloud embed code

[embed]https://soundcloud.com/globalvoices/summitpodcast[/embed]

Mostly likely this applies to other embeds as well. A general review would be greatly appreciated.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@westonruter westonruter added this to the v1.5.1 milestone Mar 26, 2020
@westonruter westonruter added this to To Do in Ongoing Mar 26, 2020
@jerclarke jerclarke changed the title amp-instagram and amp-youtube wrapped in unnecessary p tag amp-instagram, amp-youtube, amp-soundcloud wrapped in unnecessary p tags Mar 26, 2020
@pierlon pierlon moved this from To Do to In Progress in Ongoing Mar 31, 2020
@westonruter westonruter modified the milestones: v1.5.1, v1.5.2, v1.6 Mar 31, 2020
@kmyram kmyram modified the milestones: v1.6, Sprint 27 Apr 14, 2020
@kmyram kmyram added the WS:Core Work stream for Plugin core label May 15, 2020
@kmyram kmyram modified the milestones: Sprint 27, v1.6 May 27, 2020
@westonruter westonruter modified the milestones: v1.6, v1.7 Jun 26, 2020
@westonruter westonruter modified the milestones: v2.1, v2.2 Feb 11, 2021
@westonruter westonruter added the P2 Low priority label Jun 18, 2021
@westonruter westonruter modified the milestones: v2.2, v2.3 Nov 30, 2021
@westonruter westonruter modified the milestones: v2.3, v2.4 Dec 23, 2021
@westonruter westonruter removed this from the v2.4 milestone Apr 14, 2022
@jerclarke
Copy link
Author

Just here to say, would still be great to get this consistent!

Also noticed that this also applies to amp-iframe, which affects Tumblr and Mastodon 😅

@jerclarke jerclarke changed the title amp-instagram, amp-youtube, amp-soundcloud wrapped in unnecessary p tags amp-iframe, amp-instagram, amp-youtube, amp-soundcloud wrapped in unnecessary p tags Feb 2, 2023
@westonruter westonruter added this to the v2.4.2 milestone Mar 30, 2023
@thelovekesh thelovekesh linked a pull request Jun 15, 2023 that will close this issue
2 tasks
@maitreyie-chavan maitreyie-chavan removed this from the v2.4.2 milestone Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Embeds P2 Low priority Punted WS:Core Work stream for Plugin core
Projects
Ongoing
  
In Progress
6 participants