Merge pull request #671 from wordpress-mobile/issue/658-unsupported-html-fixes

UnsupportedHTML: Multiple Updates

Author: jleandroperez

Aztec/Classes/Converters/HTMLNodeToNSAttributedString.swift

@@ -216,17 +216,17 @@ private extension HTMLNodeToNSAttributedString {
             return attributes
         }
 
-        let representation = HTMLRepresentation(for: .element(HTMLElementRepresentation(element)))
+        let elementRepresentation = HTMLElementRepresentation(element)
+        let representation = HTMLRepresentation(for: .element(elementRepresentation))
         var finalAttributes = attributes
 
         if let elementFormatter = formatter(for: element) {
             finalAttributes = elementFormatter.apply(to: finalAttributes, andStore: representation)
-        } else  if element.name == StandardElementType.li.rawValue {
+        } else if element.name == StandardElementType.li.rawValue {
             // ^ Since LI is handled by the OL and UL formatters, we can safely ignore it here.
-
             finalAttributes = attributes
         } else {
-            finalAttributes = self.attributes(storing: element, in: finalAttributes)
+            finalAttributes = self.attributes(storing: elementRepresentation, in: finalAttributes)
         }
 
         for attribute in element.attributes {
@@ -270,12 +270,17 @@ private extension HTMLNodeToNSAttributedString {
     ///
     /// - Returns: A collection of NSAttributedString Attributes, including the specified HTMLElementRepresentation.
     ///
-    private func attributes(storing element: ElementNode, in attributes: [String: Any]) -> [String: Any] {
-        let unsupportedHTML = attributes[UnsupportedHTMLAttributeName] as? UnsupportedHTML ?? UnsupportedHTML()
-        unsupportedHTML.append(element: element)
-
+    private func attributes(storing representation: HTMLElementRepresentation, in attributes: [String: Any]) -> [String: Any] {
+        let unsupportedHTML = attributes[UnsupportedHTMLAttributeName] as? UnsupportedHTML
+        var representations = unsupportedHTML?.representations ?? []
+        representations.append(representation)
+
+        // Note:
+        // We'll *ALWAYS* store a copy of the UnsupportedHTML instance. Reason is: reusing the old instance
+        // would mean affecting a range that may fall beyond what we expected!
+        //
         var updated = attributes
-        updated[UnsupportedHTMLAttributeName] = unsupportedHTML
+        updated[UnsupportedHTMLAttributeName] = UnsupportedHTML(representations: representations)
 
         return updated
     }

Aztec/Classes/Converters/NSAttributedStringToNodes.swift

@@ -685,11 +685,13 @@ private extension NSAttributedStringToNodes {
     /// Extracts all of the Unsupported HTML Snippets contained within a collection of Attributes.
     ///
     private func processUnsupportedHTML(in attributes: [String: Any]) -> [ElementNode] {
-        guard let unsupported = attributes[UnsupportedHTMLAttributeName] as? UnsupportedHTML else {
+        guard let unsupportedHTML = attributes[UnsupportedHTMLAttributeName] as? UnsupportedHTML else {
             return []
         }
 
-        return unsupported.elements
+        return unsupportedHTML.representations.map { representation in
+            return representation.toElementNode()
+        }
     }
 }
 

Aztec/Classes/NSAttributedString/Styles/UnsupportedHTML.swift

@@ -10,34 +10,24 @@ let UnsupportedHTMLAttributeName = "UnsupportedHTMLAttributeName"
 //
 class UnsupportedHTML: NSObject {
 
-    /// HTML Snippets not (natively) supported by the Editor (which will be re-serialized!!)
+    /// ElementRepresentation for Unsupported HTML
     ///
-    private(set) var snippets = [String]()
+    let representations: [HTMLElementRepresentation]
 
-    /// HTML Snippets not supported, converted back to their ElementNode representations
+    /// Default Initializer
     ///
-    var elements: [ElementNode] {
-        let converter = InHTMLConverter()
-
-        return snippets.flatMap { snippet in
-            // Strip the Root Node(s): Always return the first child element
-            let root = converter.convert(snippet)
-            return root.children.first as? ElementNode
-        }
+    init(representations: [HTMLElementRepresentation]) {
+        self.representations = representations
     }
 
     /// Required Initializers
     ///
-    public required convenience init?(coder aDecoder: NSCoder) {
-        self.init()
-        self.snippets = aDecoder.decodeObject(forKey: Keys.elements) as? [String] ?? []
-    }
+    public required init?(coder aDecoder: NSCoder) {
+        guard let representations = aDecoder.decodeObject(forKey: Keys.representations) as? [HTMLElementRepresentation] else {
+            return nil
+        }
 
-    /// Appends the specified Element Representation
-    ///
-    func append(element: ElementNode) {
-        let snippet = OutHTMLConverter().convert(element)
-        snippets.append(snippet)
+        self.representations = representations
     }
 }
 
@@ -47,10 +37,10 @@ class UnsupportedHTML: NSObject {
 extension UnsupportedHTML: NSCoding {
 
     struct Keys {
-        static let elements = "elements"
+        static let representations = "representations"
     }
 
     open func encode(with aCoder: NSCoder) {
-        aCoder.encode(snippets, forKey: Keys.elements)
+        aCoder.encode(representations, forKey: Keys.representations)
     }
 }

AztecTests/Converters/HTMLNodeToNSAttributedStringTests.swift

@@ -33,24 +33,26 @@ class HTMLNodeToNSAttributedStringTests: XCTestCase {
             return
         }
 
+        let representations = unsupportedHTML.representations
         XCTAssert(range.length == textNode.length())
-        XCTAssert(unsupportedHTML.elements.count == 2)
+        XCTAssert(representations.count == 2)
 
-        let restoredSpanElement2 = unsupportedHTML.elements.last
+        let restoredSpanElement2 = representations.last
         XCTAssertEqual(restoredSpanElement2?.name, "span")
 
         let restoredSpanAttribute2 = restoredSpanElement2?.attributes.first
         XCTAssertEqual(restoredSpanAttribute2?.name, "class")
         XCTAssertEqual(restoredSpanAttribute2?.value.toString(), "aztec")
 
-        let restoredSpanElement1 = unsupportedHTML.elements.first
+        let restoredSpanElement1 = representations.first
         XCTAssertEqual(restoredSpanElement1?.name, "span")
 
         let restoredSpanAttribute1 = restoredSpanElement1?.attributes.first
         XCTAssertEqual(restoredSpanAttribute1?.name, "class")
         XCTAssertEqual(restoredSpanAttribute1?.value.toString(), "first")
     }
 
+
     /// Verifies that the DivFormatter effectively appends the DIV Element Representation, to the properties collection.
     ///
     func testHtmlDivFormatterEffectivelyAppendsNewDivProperty() {
@@ -95,6 +97,41 @@ class HTMLNodeToNSAttributedStringTests: XCTestCase {
         XCTAssert(restoredDiv3.name == divNode3.name)
         XCTAssert(restoredDiv3.attributes == [divAttr3])
     }
+
+
+    /// Verifies that BR elements contained within div tags do not cause any side effect.
+    /// Ref. #658
+    ///
+    func testLineBreakTagWithinHTMLDivGetsProperlyEncodedAndDecoded() {
+        let inHtml = "<div><br>Aztec, don't forget me!</div>"
+
+        let inNode = InHTMLConverter().convert(inHtml)
+        let attrString = attributedString(from: inNode)
+
+        let outNode = NSAttributedStringToNodes().convert(attrString)
+        let outHtml = OutHTMLConverter().convert(outNode)
+
+        XCTAssertEqual(outHtml, inHtml)
+    }
+
+
+    /// Verifies that BR elements contained within span tags do not cause Data Loss.
+    /// Ref. #658
+    ///
+    func testLineBreakTagWithinUnsupportedHTMLDoesNotCauseDataLoss() {
+        let inHtml = "<span><br>Aztec, don't forget me!</span>"
+        let expectedHtml = "<br><span>Aztec, don't forget me!</span>"
+// TODO: The actual expected html should wrap the BR within a span tag. To be addressed in another PR!
+//        let expectedHtml = "<span><br></span><span>Aztec, don't forget me!</span>"
+
+        let inNode = InHTMLConverter().convert(inHtml)
+        let attrString = attributedString(from: inNode)
+
+        let outNode = NSAttributedStringToNodes().convert(attrString)
+        let outHtml = OutHTMLConverter().convert(outNode)
+
+        XCTAssertEqual(outHtml, expectedHtml)
+    }
 }
 
 
@@ -103,7 +140,7 @@ class HTMLNodeToNSAttributedStringTests: XCTestCase {
 extension HTMLNodeToNSAttributedStringTests {
 
     func attributedString(from node: Node) -> NSAttributedString {
-        let descriptor = UIFont.boldSystemFont(ofSize: 14).fontDescriptor
+        let descriptor = UIFont.systemFont(ofSize: 14).fontDescriptor
         let converter = HTMLNodeToNSAttributedString(usingDefaultFontDescriptor: descriptor)
 
         return converter.convert(node)

AztecTests/Converters/NSAttributedStringToNodesTests.swift

@@ -615,11 +615,11 @@ class NSAttributedStringToNodesTests: XCTestCase {
         let testingString = NSMutableAttributedString(string: text)
 
         let spanElement = ElementNode(type: .span)
+        let representation = HTMLElementRepresentation(spanElement)
 
-        let unsupported = UnsupportedHTML()
-        unsupported.append(element: spanElement)
-
-        testingString.addAttribute(UnsupportedHTMLAttributeName, value: unsupported, range: testingString.rangeOfEntireString)
+        // Store
+        let unsupportedHTML = UnsupportedHTML(representations: [representation])
+        testingString.addAttribute(UnsupportedHTMLAttributeName, value: unsupportedHTML, range: testingString.rangeOfEntireString)
 
         // Convert + Verify
         let node = NSAttributedStringToNodes().convert(testingString)

AztecTests/TextKit/UnsupportedHTMLTests.swift

@@ -9,22 +9,18 @@ class UnsupportedHTMLTests: XCTestCase {
     /// Verifies that a UnsupportedHTML Instance can get properly serialized back and forth
     ///
     func testSnippetsGetProperlyEncodedAndDecoded() {
-        let unsupported = UnsupportedHTML()
-        unsupported.append(element: sampleElement)
-        unsupported.append(element: sampleElement)
+        let unsupported = UnsupportedHTML(representations: [sampleRepresentation, sampleRepresentation])
 
         let data = NSKeyedArchiver.archivedData(withRootObject: unsupported)
         guard let restored = NSKeyedUnarchiver.unarchiveObject(with: data) as? UnsupportedHTML else {
             XCTFail()
             return
         }
 
-        let elements = restored.elements
-        XCTAssert(elements.count == 2)
+        XCTAssert(restored.representations.count == 2)
 
-        for element in elements {
-            XCTAssert(element.children == sampleChildren)
-            XCTAssert(element.attributes == sampleAttributes)
+        for representation in restored.representations {
+            XCTAssert(representation == sampleRepresentation)
         }
     }
 }
@@ -55,4 +51,8 @@ private extension UnsupportedHTMLTests {
     var sampleElement: ElementNode {
         return ElementNode(name: "Test", attributes: self.sampleAttributes, children: self.sampleChildren)
     }
+
+    var sampleRepresentation: HTMLElementRepresentation {
+        return HTMLElementRepresentation(self.sampleElement)
+    }
 }