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

Twitter embed fails with trailing slash on URL #12664

Closed
kriskarkoski opened this issue Dec 7, 2018 · 7 comments
Closed

Twitter embed fails with trailing slash on URL #12664

kriskarkoski opened this issue Dec 7, 2018 · 7 comments
Labels
[Block] Embed Affects the Embed Block Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended
Milestone
WordPress 5.x

Comments

@kriskarkoski
Copy link

Describe the bug
Attempting to embed a Tweet with a URL containing a trailing slash causes the embed to fail. While Twitter seems to default to no-slash it does not redirect a URL with a trailing slash to the no-slash version.

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'wp-admin/post-new.php'
  2. Paste a Twitter single-Tweet URL with a trailing slash on its own or in the embed or Twitter blocks
  3. See embed error and URL displays on the front-end instead of embed

Expected behavior
The Tweet to embed the same as with no trailing slash

Screenshots
screen shot 2018-12-06 at 7 43 55 pm

Desktop (please complete the following information):

  • OS: OS X 10
  • Chrome
  • Version 70.0.3538.11

Additional context

  • WordPress 5.0. No plugins.
@danielbachhuber danielbachhuber added [Type] Bug An existing feature does not function as intended [Block] Embed Affects the Embed Block Good First Issue An issue that's suitable for someone looking to contribute for the first time labels Dec 7, 2018
@danielbachhuber
Copy link
Member

Thanks for the report, @kriskarkoski. This should be relatively straightforward to fix up. I've filed it against WordPress 5.0.x Follow Ups

@jeherve jeherve mentioned this issue Dec 7, 2018
Closed
@earnjam earnjam added the [Status] In Progress Tracking issues with work in progress label Dec 9, 2018
@alpinealex alpinealex mentioned this issue Dec 9, 2018
Closed
4 tasks
@gziolo gziolo removed the [Status] In Progress Tracking issues with work in progress label Feb 7, 2019
@gziolo
Copy link
Member

gziolo commented Feb 7, 2019

Initial exploration was executed by @alpinealex in #12739. It wasn't finished and this issue can be taken over by someone else. This is the latest recommendation from @notnownikki how to fix this bug:

I'd rather this removed the trailing slash if the embed failed, just so we can support embed URLs that do end with a trailing slash. That should be easy enough to do in componentDidUpdate (of edit component).

@gbroques
Copy link

@gziolo I can take this over.

@gziolo
Copy link
Member

gziolo commented Mar 24, 2019

@gbroques, thank you 👍

@gbroques
Copy link

gbroques commented Mar 24, 2019 •
edited
Loading

@gziolo I believe I fixed the issue as @notnownikki suggested and put up a PR #14600.

However, I can't seem to run the e2e tests locally to fix the test that's failing the build.

Error: The constant 'SCRIPT_DEBUG' is not defined in the 'wp-config.php' file.

I'm using the built-in local environment as suggested by the End to end testing section in the docs.

OS: Ubuntu 16.04
Docker version: 18.09.3, build 774a1f4
Docker Compose version: 1.22.0, build f46880fe
Node version: v10.15.3

@notnownikki
Copy link
Member

notnownikki commented Mar 24, 2019 via email

@gbroques gbroques mentioned this issue Mar 25, 2019
Closed
5 tasks
gbroques added a commit to gbroques/gutenberg that referenced this issue Mar 28, 2019
@gbroques
Fix embedding Twitter URLs with a trailing slash (Closes WordPress#12664
9f0f0ce 
)
@gbroques
Copy link

Another contributor can pick this issue up and use my PR #14600 as a starting place.

It looks like all that's needed is a fix to the e2e tests. Read the PR comments for @notnownikki's suggestion.

Hope this helps! :)

notnownikki pushed a commit that referenced this issue Mar 29, 2019
@gbroques @notnownikki
Fix embedding Twitter URLs with a trailing slash (Closes #12664)
aea4cef
@notnownikki notnownikki mentioned this issue Mar 29, 2019
Merged
5 tasks
notnownikki added a commit that referenced this issue Mar 29, 2019
@notnownikki
Retry failing embeds with trailing slash (#14705)
fd72892 
* Fix embedding Twitter URLs with a trailing slash (Closes #12664)

* Fix race condition for WordPress URLs that end in slashes, add test
ellatrix pushed a commit that referenced this issue Apr 3, 2019
@notnownikki @ellatrix
Retry failing embeds with trailing slash (#14705)
96ea2a2 
* Fix embedding Twitter URLs with a trailing slash (Closes #12664)

* Fix race condition for WordPress URLs that end in slashes, add test
ellatrix added a commit that referenced this issue Apr 4, 2019
@ellatrix
Bug fixes from 5.4 (#14796)
09ac4a5 
* RichText: improve format boundary style (#14519)

* RichText: improve format boundary style

* rgb => rgba

* Paste: check plain text for gutenberg content (#14536)

* Make ClipboardButton inside a block work correctly in Safari (#7106)

* Make ClipboardButton inside a block work in Safari

* Update changelogs

* Block Editor: Update "Next" to "Unreleased" per guidelines

https://github.com/WordPress/gutenberg/blob/master/packages/README.md#maintaining-changelogs

* Input Interaction: always expand single line selection vertically (#14487)

* Input Interaction: always expand single line selection vertically

* Add e2e test

* Use MenuItem instead of IconButton (#14569)

* Remove id, infoid, label and aria-describedby from MenuItem (#14423)

* Preformatted: save line breaks as characters (#14653)

* Preformatted: save line breaks as characters

* Update e2e test

* Remove negative toolbar position rules from full-aligned blocks. (#14669)

* Fix issue with double scrollbar in Fullscreen Mode (#14677)

This PR fixes an issue where the sidebar would have two scrollbars when in fullscreen mode.

* Fix WordPress embed block resolution (#14658)

* Retry failing embeds with trailing slash (#14705)

* Fix embedding Twitter URLs with a trailing slash (Closes #12664)

* Fix race condition for WordPress URLs that end in slashes, add test

* API Fetch: Fix error on empty OPTIONS preload data (#14714)

* Input Interaction: better horizontal edge detection (#14462)

* Input Interaction: better horizontal edge detection

* Correct BR ranges

* Add e2e test

* Increase buffer for Firefox

* Clean up

* Merge isEdge logic

* Fix typo

* Address feedback

* Build docs

* Fix memize option key typo (#14750)

* RichText: unify active formats, 'selectedFormat' and 'placeholderFormat' (#14411)

* RichText: unify active formats, 'selectedFormat' and 'placeholderFormat'

* Add extra e2e test

* Only should boundary style when focused

* Update docs

* Try to trigger tests with Travis

* Restore Travis config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Block] Embed Affects the Embed Block Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
WordPress 5.x
7 participants
@danielbachhuber @youknowriad @gziolo @kriskarkoski @notnownikki @earnjam @gbroques