Make WordPress Core

Opened 3 years ago

Closed 19 months ago

Last modified 19 months ago

#53404 closed enhancement (fixed)

Replace http with https entering a video link

Reported by: presskopp's profile Presskopp Owned by: joedolson's profile joedolson
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch commit
Focuses: administration Cc:

Description

Using the classic editor / classic block:

1) Add Media
2) Insert from URL

A field appears prefilled with "http://"

Just like in #46312 this should be changed to "https://" (or even not be prefilled at all)

Some quotes from slack(core-media):

@adamsilverstein:

https as the default suggested prefix makes sense to me

@joyously:

even better is to match the placeholder to the scheme of the site

@joedolson:

If there's going to be a default, I think that https:// makes the most sense, but I'm not sure there should be a default at all. Perhaps there should be a placeholder that defaults to the site's protocol, but I don't think any content should be prefilled.

Using Gutenberg:

1) Add Video Block
2) Insert from URL

A field appears having no prefilled value

---

Besides of this there's more to consider:

Using the classic editor / classic block:

During typing WP already tries to make a connection to the entered URL. In case the site uses https, if http is used, the browser (chrome used here) throws a warning about mixed content in console and switches automatically to https.
ALSO: If entered a decimal URL a conversion takes place, for example http://134744072 will be converted to http://8.8.8.8/
Whatever will be entered, the console fills up with warnings BUT there will be no warning or error visible in the backend. I can enter whatever and it will be pasted as Video-URL unchanged.
So we don't need those connection attempts at all here.

Using Gutenberg:

In Gutenberg the URL will only be checked after pressing enter.

Attachments (1)

53404.2.diff (547 bytes) - added by joedolson 19 months ago.
Set placeholder instead of value when model has no data

Download all attachments as: .zip

Change History (10)

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


23 months ago

#2 @joedolson
23 months ago

  • Focuses administration added
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.1
  • Owner set to joedolson
  • Status changed from new to accepted

This ticket was mentioned in PR #3241 on WordPress/wordpress-develop by thijsoo.


19 months ago
#3

  • Keywords has-patch added; needs-patch removed

Replaced the http placeholder for the classic video menu with https
Trac ticket: https://core.trac.wordpress.org/ticket/53404

@joedolson
19 months ago

Set placeholder instead of value when model has no data

#4 @joedolson
19 months ago

I still think that using a placeholder would be better than value when there's no data; the updated patch here does that instead.

Based on the terminology used in the PR, I think that this already understood as a placeholder, it just needs to *actually* be a placeholder rather than a value.

#5 @costdev
19 months ago

@joedolson I'm inclined to agree. Displaying https:// as a placeholder indicates that a protocol will be required for a valid URL, while also not getting in the way of pasting a link, which often already includes https://. The patch looks good to me.

Do you still think we should set the placeholder to the same protocol as the site, or should that be considered for a later update? I'm mindful that 6.1 Beta 1 is tomorrow.

#6 @joedolson
19 months ago

I think we should stick with https:// in either circumstance. Embedding an https resource on an http page doesn't cause any problems, but the reverse isn't true, so it's generally going to be safer to encourage https, and it doesn't prevent any behavior in either case. Since it's pretty much always going to be better for an embedded resource to be sourced from https, I can't see any good argument for using http.

#7 @joedolson
19 months ago

  • Keywords commit added

Marking for commit.

#8 @joedolson
19 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 54228:

Media: Set https as placeholder for embedding media from URL.

Change the "Insert from URL" field in the media library to use a placeholder suggesting the https:// protocol instead of a value attribute with http://.

Props Presskopp, adamsilverstein, joyously, thijso, joedolson, costdev.
Fixes #53404.

#9 @costdev
19 months ago

@joedolson I can't see any good argument for using http.

I agree!

Note: See TracTickets for help on using tickets.