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.

Sign up for GitHub

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

Jump to bottom

Issue/1045 paste of attributed strings #1097

Merged
merged 11 commits into from
Jun 10, 2019
Merged

Issue/1045 paste of attributed strings #1097

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
08e996e
Update Aztec reference.
SergioEstevao Jun 7, 2019
f9186ca
Convert RFT from pasteboard to HTML.
SergioEstevao Jun 7, 2019
440e60c
Improve paste code to read more UTI when pasting HTML or text.
SergioEstevao Jun 7, 2019
4580317
Remove non utf8 paste retrieval.
SergioEstevao Jun 7, 2019
d181c81
Update Aztec to 1.7.0
SergioEstevao Jun 7, 2019
f9eef7b
Simplify string retrieval from pasteboard.
SergioEstevao Jun 7, 2019
eb23f13
Merge branch 'develop' into issue/1045_paste_of_attributed_strings
SergioEstevao Jun 7, 2019
0061b1e
Reorder the order where rich text is pasted.
SergioEstevao Jun 7, 2019
f75b3c1
Make sure HTML imported is not full HTML document with styles and oth…
SergioEstevao Jun 7, 2019
26883a1
Refactor method to make it easier to use.
SergioEstevao Jun 10, 2019
157345c
Merge branch 'develop' into issue/1045_paste_of_attributed_strings
SergioEstevao Jun 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion react-native-aztec/ios/Cartfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
github "wordpress-mobile/AztecEditor-iOS" "fff3d4712c6971cd2673fce273c8e9d9330a15ca"
github "wordpress-mobile/AztecEditor-iOS" "1.7.0"
2 changes: 1 addition & 1 deletion react-native-aztec/ios/Cartfile.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
github "wordpress-mobile/AztecEditor-iOS" "fff3d4712c6971cd2673fce273c8e9d9330a15ca"
github "wordpress-mobile/AztecEditor-iOS" "1.7.0"
60 changes: 40 additions & 20 deletions react-native-aztec/ios/RNTAztecView/RCTAztecView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,26 +178,41 @@ class RCTAztecView: Aztec.TextView {
}

// MARK: - Paste handling

private func html(from pasteboard: UIPasteboard) -> String? {
guard let data = pasteboard.data(forPasteboardType: kUTTypeHTML as String) else {
return nil
private func read(from pasteboard: UIPasteboard, uti: CFString, documentType: DocumentType) -> String? {
guard let data = pasteboard.data(forPasteboardType: uti as String),
Copy link
Contributor

Choose a reason for hiding this comment

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

Small detail:
uti is already String so this extra as String is not needed.

Maybe we can change uti's type to be CFString to make it simpler on the caller side?

let attributedString = try? NSAttributedString(data: data, options: [.documentType: documentType], documentAttributes: nil),
let storage = self.textStorage as? TextStorage else {
return nil
}

return String(data: data, encoding: .utf8)
return storage.getHTML(from: attributedString)
}

private func text(from pasteboard: UIPasteboard) -> String? {
guard let data = pasteboard.data(forPasteboardType: kUTTypePlainText as String) else {
return nil

private func readHTML(from pasteboard: UIPasteboard) -> String? {

if let data = pasteboard.data(forPasteboardType: kUTTypeHTML as String), let html = String(data: data, encoding: .utf8) {
// Make sure we are not getting a full HTML DOC. We only want inner content
if !html.hasPrefix("<!DOCTYPE html") {
Copy link
Contributor Author

@SergioEstevao SergioEstevao Jun 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I'm doing this check here to check if we are not importing a full HTML document because on my tests I found that the when copying from the Notes app, it was sending the full HTML document, using span and styles for bold/italic styles instead of using just <strong> and <em> element.

My assumption is that when importing HTML if a full document is there, maybe there is content there is not valid or importable to GB so instead we should try to read RTF.

If we think that this is not a good assumption we can remove this line, but then import from the Notes app will not work.

For the record when pasting from Safari, it provide only the selected HTML and not the full document so the import from HTML works correctly.

Copy link
Contributor

@mkevins mkevins Jun 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

using span and styles for bold/italic styles instead of using just <strong> and <em> element.

My assumption is that when importing HTML if a full document is there, maybe there is content there is not valid or importable to GB so instead we should try to read RTF.

FWIW, code exists on GB side to handle inline styles on span tags, however, the limitations of our DOM implementation (jsdom-jscore) prevent it from working correctly.

To "enable" this code path, we'd need the style property on ELEMENT_NODEs to be implemented, as well as ATTRIBUTE_NODEs.

If we think that this is not a good assumption we can remove this line, but then import from the Notes app will not work.

It's possible that implementing the necessary DOM spec will require significant time / effort. I think your approach is good to mitigate the underlying problem (that our DOM implementation is insufficient). I wonder if wrapping the html in a "full document" is correlated with using inline styles on spans, or if Notes.app happens to behave this way, and other apps have differing behavior regarding these two characteristics.

Is there some way to directly check if the pasteboard html generated by the external app uses inline styles on span tags?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think your approach is good to mitigate the underlying problem

I agree with this!

I wonder if wrapping the html in a "full document" is correlated with using inline styles on spans, or if Notes.app happens to behave this way, and other apps have differing behavior regarding these two characteristics.

I just tested the Google docs app, and it happens that it does use inline styles on spans, but without wrapping it up on a full HTML document. So this approach doesn't work for it.

But it does work for the Notes app.

I'm OK with shipping this improvement to support the Apple default's Note app, and check for a more wide alternative later on.

What do you think @SergioEstevao ?

return html
}
}

return String(data: data, encoding: .utf8)
}

private func cleanHTML() -> String {
let html = getHTML(prettify: false)
return html
if let flatRTFDString = read(from: pasteboard, uti: kUTTypeFlatRTFD, documentType: DocumentType.rtfd) {
return flatRTFDString
}

if let rtfString = read(from: pasteboard, uti: kUTTypeRTF, documentType: DocumentType.rtf) {
return rtfString
}

if let rtfdString = read(from: pasteboard, uti: kUTTypeRTFD, documentType: DocumentType.rtfd) {
return rtfdString
}

return nil
}

private func readText(from pasteboard: UIPasteboard) -> String? {
return pasteboard.string
}

func saveToDisk(image: UIImage) -> URL? {
Expand All @@ -216,7 +231,7 @@ class RCTAztecView: Aztec.TextView {
return fileURL
}

private func images(from pasteboard: UIPasteboard) -> [String] {
private func readImages(from pasteboard: UIPasteboard) -> [String] {
guard let images = pasteboard.images else {
return []
}
Expand All @@ -229,9 +244,9 @@ class RCTAztecView: Aztec.TextView {
let end = selectedRange.location + selectedRange.length

let pasteboard = UIPasteboard.general
let text = self.text(from: pasteboard) ?? ""
let html = self.html(from: pasteboard) ?? ""
let imagesURLs = self.images(from: pasteboard)
let text = readText(from: pasteboard) ?? ""
let html = readHTML(from: pasteboard) ?? ""
let imagesURLs = readImages(from: pasteboard)

onPaste?([
"currentContent": cleanHTML(),
Expand Down Expand Up @@ -315,6 +330,11 @@ class RCTAztecView: Aztec.TextView {
}

// MARK: - Native-to-RN Value Packing Logic

private func cleanHTML() -> String {
let html = getHTML(prettify: false)
return html
}

func packForRN(_ text: String, withName name: String) -> [AnyHashable: Any] {
return [name: text,
Expand Down