Conversation
ibexa-workflow-automation-1
bot
requested review from
Steveb-p,
ViniTou,
adamwojs,
alongosz,
barw4,
ciastektk,
konradoboza,
mikadamczyk,
tbialcz and
wiewiurdp
and removed request for
a team
reithor
force-pushed
the
ibx-10978-add-tel-mailto-to-urltype
branch
from
a7b72e4 to
12c8017
Compare
|
|
@alongosz : https://www.rfc-editor.org/rfc/rfc1738 https://url.spec.whatwg.org/#urls |
|
Hi @alongosz any chances of moving forward with the review? :) |
alongosz
left a comment
alongosz
left a comment
There was a problem hiding this comment.
Hi @alongosz any chances of moving forward with the review? :)
Hi @mateuszbieniek and @reithor, my apologies, looks like I made final review remarks, but I never submitted them 🤦
Here are some last nitpicks:
| /** @var string|null */ | ||
| private $defaultProtocol; | ||
|
|
||
| /** @var \Symfony\Component\Form\Extension\Core\EventListener\FixUrlProtocolListener */ | ||
| private $fixUrlProtocolListener; |
There was a problem hiding this comment.
Please use PHP 7.4 strict types.
| /** | ||
| * @param string|null $defaultProtocol The URL scheme to add when there is none or null to not modify the data | ||
| */ | ||
| public function __construct(?string $defaultProtocol = 'http') |
There was a problem hiding this comment.
Can this be https by default or does it somehow influence BC?
4 participants
Related PRs:
Description:
PR disables Symfony's
FixUrlProtocolListenerand introduces a customFixUrlProtocolListener.Symfony's
FixUrlProtocolListenerconsiders URLs liketel:123456(without//) as having no protocol.Those URLs will wrongly be prefixed with
http://.(Technically URLs like
tel:123456are valid).Own implementation fixes this behavior.
For now only
telandmailtoare added as allowed scheme - further improvement could make this part more flexible via config,Reference for URL syntax:
https://en.wikipedia.org/wiki/URL#Syntax
For QA:
Documentation: