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

Allow custom url schemes #9538

Merged
merged 3 commits into from
Nov 11, 2019
Merged

Allow custom url schemes #9538

merged 3 commits into from
Nov 11, 2019

Conversation

bleistivt
Copy link
Contributor

I followed the suggestions of #7769

The only difference is, that Garden.Html.CustomUrlSchemes replaces the default config in my implementation. This is because afaik array configs are somewhat deprecated and because of HtmLawed's special scheme syntax which doesn't really map to an array. String concatenation would be another option.

Copy link
Contributor

@charrondev charrondev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we'd want to concat the default ones as that prevent people from removing allowed schemes from their config.

There are 2 changes I would request though, mostly because I'd prefer to be able to re-use the same config value in Rich Editor in the future.

  • Make the config value Garden.Html.AllowedUrlSchemes.
  • Have the config value be an array of strings (was there somewhere you saw referenced disallowing arrays in the config? I think it's only serialized arrays that are being discontinued, but maybe @tburry knows better). I know for sure use an array for Garden.Uploads.AllowedFileExtensions and I don't think that's going anywhere anytime soon.
  • Join that array into a string to insert into this rule.

@bleistivt
Copy link
Contributor Author

Based on this comment, but it is quite old:
#4043 (comment)

The problem is that the HtmLawed config defines multiple groups:

classid: clsid;
href: aim, feed, file, ftp, gopher, http, https, irc, mailto, news, nntp, rapidminer, sftp, ssh, telnet;
style: nil;
*: file, http, https'

http://www.bioinformatics.org/phplabware/internal_utilities/htmLawed/htmLawed_README.htm#s3.4.3

Do you only want the href group to become Garden.Html.AllowedUrlSchemes?

@charrondev
Copy link
Contributor

Do you only want the href group to become Garden.Html.AllowedUrlSchemes?

That's what I was thinking. Do you have any need of specifying the other ones? I feel like as a config setting it would be more useful if it didn't have to be based around Htmlawed's specific syntax.

Rich editor for example, would like to use a shared config setting, and that doesn't use Htmlawed at all.

@charrondev charrondev merged commit d8f7839 into vanilla:master Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants