-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Share Extension: Improve text import from other apps #11288
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking fantastic! Fixes the empty editor issue for me when sharing text, and looks easily extendible to add other formats in the future.
Just two small code comments from me!
} else { | ||
returnString = "<blockquote><p>\(selectedText)\(readOnText)</p></blockquote>" | ||
guard importedText.isEmpty else { | ||
return "<p>\(importedText.escapeHtmlNamedEntities())</p>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have imported text, isn't it likely that it's going to be more than one paragraph? Do we need to wrap it in tags here?
@@ -195,6 +205,7 @@ private extension TypeBasedExtensionContentExtractor { | |||
|
|||
func extract(context: NSExtensionContext, completion: @escaping ([ExtractedItem]) -> Void) { | |||
let itemProviders = context.itemProviders(ofType: acceptedType) | |||
print(acceptedType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to log this out, should we use DDLog
instead of print
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I meant to remove that entirely 😉
@frosty ready for re-review! |
This is hands down awesome. About the HTML, I may be missing a use case here but I wonder if escaping it the correct approach instead of converting it to an attributed string and using it in the editor. This approach will eventually allow passing images as base64 in the HTML document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
@trix180 Thanks for the HTML –> Attributed String idea... we'll investigate! |
Merging as Nate is AFK this week. |
Fixes #11222
Fixes #11270
This fixes text import from other applications by treating text from shared files differently than from text shared directly (which will still be blockquoted). It also fixes the issue of shared text (both kinds) being treated like HTML, opening the possibility of nefarious HTML making it into the user's site.
To test:
Update release notes:
RELEASE-NOTES.txt
.