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

Using tel: on redirector pages #1937

Open
matthewalkr opened this issue Aug 24, 2017 · 11 comments
Open

Using tel: on redirector pages #1937

matthewalkr opened this issue Aug 24, 2017 · 11 comments

Comments

@matthewalkr
Copy link

matthewalkr commented Aug 24, 2017

We have auxiliary navigation rails that include both links and phone numbers such as tel:0800123456 based on the RedirectorPage. This update "[SS-2017-003] Only allow HTTP(S) links for external redirector pages" breaks that and broke our web app.

In my view, limiting the schemes to HTTP and HTTPS is too restrictive. Why not allow other schemes such as TEL, and given the proliferation of schemes, why not generalise?

Here's the relevant commit

@tractorcow
Copy link
Contributor

I'd be happy with allowed_schemas to be introduced to RedirectorPage, to make it user manageable. Do you feel like writing a PR for introduce this? (I suggest the 3 branch).

@matthewalkr
Copy link
Author

Yeah I can give it a crack...Thanks for your thoughts.

@dhensby
Copy link
Contributor

dhensby commented Aug 25, 2017

Whilst I'd support adding a config list of allowed schemes I'd say that using redirector pages to "redirect" to tel: links is an abuse of the RedirectorPage.

I'd urge you to either consider creating a specific page type for this or introducing a menu manager module to allow arbitrary links in menus.

@matthewalkr
Copy link
Author

What is the imagined use case of RedirectorPage? Is it the ability to add something arbitrary to the page tree? As that's what I'm doing. Genuine question as I don't want to propose changes that are niche to my needs. I have already worked around the issue.

A menu manager would be better but there are currently none that meet our needs. I might work on that in future but time does not allow at the moment.

@dhensby
Copy link
Contributor

dhensby commented Aug 30, 2017

Is it the ability to add something arbitrary to the page tree?

No, its the ability to add a page to the site tree that transparently points to another page (be it on the site or external).

A menu manager would be better but there are currently none that meet our needs. I might work on that in future but time does not allow at the moment.

I don't know what your requirements are, but https://github.com/heyday/silverstripe-menumanager is basically a menu built entirely of redirector page type objects...

@matthewalkr
Copy link
Author

OK cheers. My position is that web pages are no different from any other legitimate URI scheme referenced here https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml#uri-schemes-1 . I don't see why SilverStripe needs to be opinionated in this matter.

Thanks for the MenuManager note. I like that app but I don't think it integrates with Translatable.

As I say, I've worked around this issue, so it's not blocking me. More a philosophical query.

@tractorcow
Copy link
Contributor

Does tel: protocol get natively interpreted as "show the dialpad" on mobile devices, or other similarly useful (if strictly misuse) behaviour?

@matthewalkr
Copy link
Author

It will actually dial the number (given a prompt first). In my view, it is the responsibility of the browser to handle the link (or not) properly.

Here's an example in the wild. On desktop, not the auxiliary menu above the main nav. This menu comprises key conversion actions. On mobile it's moved under the main nav... https://www.rainbowsprings.co.nz/

@tractorcow
Copy link
Contributor

I'm in favour of updating RedirectorPage so that support for custom protocols COULD be added in. E.g. via an extension on RedirectorPage. That way the responsibility for deciding whether to follow the rules is up to usercode. :)

Perhaps a validateUrl, with an $this->extend('updateValidateUrl', $url, $valid) style approach?

@axllent
Copy link
Contributor

axllent commented Sep 1, 2017

Would this issue cover support for tel: links from within TinyMCE, or should that be opened as a separate issue? Currently the "link to external URL" from within the editor appends http:// to any tel:<nr>, possibly because they don't contain the // after the URI Scheme.

@tractorcow
Copy link
Contributor

No this issue is specific to redirectorpages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants