-1

I have a very simple UIViewController subclass which configures its view in viewDidLoad:

class TextViewController: UIViewController {

    private var textView: UITextView?

    var htmlText: String? {
        didSet {
            updateTextView()
        }
    }

    private func updateTextView() {
        textView?.setHtmlText(htmlText)
    }

    override func viewDidLoad() {
        super.viewDidLoad()

        // Do any additional setup after loading the view.
        textView = UITextView()

        // add as subview, set constraints etc.

        updateTextView()
    }
}

(.setHtmlText is an extension on UITextView which turns HTML into an NSAttributedString, inspired by this answer)

An instance of TextViewController is created, .htmlText is set to "Fetching...", an HTTP request is made and the viewcontroller is pushed onto a UINavigationController.

This results in a call to updateTextView which has no effect (.textView is still nil), but viewDidLoad ensures the current text value is shown by calling it again. Shortly afterwards, the HTTP request returns a response, and .htmlText is set to the body of that response, resulting in another call to updateTextView.

All of this code is run on the main queue (confirmed by setting break points and inspecting the stack trace), and yet unless there is a significant delay in the http get, the final text displayed is the placeholder ("Fetching..."). Stepping through in the debugger reveals that the sequence is:

1. updateTextView()     // htmlText = "Fetching...", textView == nil
2. updateTextView()     // htmlText = "Fetching...", textView == UITextView
3. updateTextView()     // htmlText = <HTTP response body>
4. setHtmlText(<HTTP response body>)
5. setHtmlText("Fetching...")

So somehow the last call to setHtmlText appears to overtake the first. Similarly bizarrely, looking back up the call stack from #5, while setHtmlText is claiming that it was passed "Fetching...", it's caller believes it's passing the HTTP HTML body.

Changing the receiver of the HTTP response to do this:

DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { vc.htmlText = html }

Rather than the more conventional:

DispatchQueue.main.async { vc.htmlText = html }

... does result in the expected final text being displayed.

All of this behaviour is reproducible on simulator or real device. A slightly hacky feeling "solution" is to put another call to updateTextView in viewWillAppear, but that's just masking what's going on.

Edited to add:

I did wonder whether it was adequate to just have one call to updateTextView in viewWillAppear, but it needs to be called from viewDidLoad AND viewWillAppear for the final value to be displayed.

Edited to add requested code:

let theVc = TextViewController()

theVc.htmlText = "<i>Fetching...</i>"

service.get(from: url) { [weak theVc] (result: Result<String>) in

    // DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) {
    DispatchQueue.main.async {
        switch result {
        case .success(let html):
            theVc?.htmlText = html
        case .error(let err):
            theVc?.htmlText = "Failed: \(err.localizedDescription)"
        }
    }
}

navigationController.pushViewController($0, animated: true)

Edited to add simplified case, eliminating the HTTP service, with the same behaviour:

let theVc = TextViewController()

theVc.htmlText = "<i>Before...</i>"

DispatchQueue.main.async {
    theVc.htmlText = "<b>After</b>"
}

navigationController.pushViewController(theVc, animated: true)

This yields an equivalent sequence of calls to updateTextView() as before:

  1. "Before", no textView yet
  2. "Before"
  3. "After"

And yet "Before" is what I see on-screen.

Setting a break point at the start of setHtmlText ("Before") and stepping through reveals that while the first pass is in NSAttributedString(data:options:documentAttributes:) the run-loop is re-entered and the second assignment ("After") is given chance to run to completion, assigning it's result to .attributedText. Then, the original NSAttributedString is given chance to complete and it immediately replaces .attributedText.

This is a quirk of the way NSAttributedStrings are generated from HTML (see somebody having similar issues when populating a UITableView)

Chris
  • 3,445
  • 3
  • 22
  • 28
  • I don't think it's realted to AttributedText as you said, but more about the async stuff. Could you show the whole part of it? When you do the call, how you manage it, etc. – Larme Mar 28 '19 at 11:05
  • @Larme Requested code added above – Chris Mar 28 '19 at 11:13
  • I think, You should call the service in the `viewDidLoad` of the `TextViewController`. – hardik parmar Mar 28 '19 at 13:29
  • Where is the code for `setHtmlText`? You claim the problem lies here but you do not show us the code??? – matt Mar 28 '19 at 15:23
  • @matt - The question links to it (https://stackoverflow.com/a/30711288/325366). Also, see the end of the question where I discuss the discovery of `NSAttributedString(data:options:documentAttributes:)` having/entering a run-loop and link to somebody else experiencing the same problem. It's a quirk of that initialiser. – Chris Mar 28 '19 at 15:35
  • I don’t care what the question links it to! I’m asking you to show me _your actual code_. I went to a lot of trouble to reproduce what you’re doing; you owe it to your readers to help them do that. – matt Mar 28 '19 at 15:56
  • Sorry Matt. Your effort is appreciated and I certainly didn't mean to cause you extra work. I've been jumping around responding to the comments on this question, whilst experimenting on solutions and adding new findings to the question. My previous comment should perhaps have more explicit in saying "I've discovered that the NSAttributedString html initialiser is the cause, no further experiments required". I'm not certain of the ordering of who said what where, but I did add that to the question a while ago, but apologies once more if that wasn't clear. – Chris Mar 28 '19 at 16:07

2 Answers2

1

I solved this by eliminating your extension and simply writing the code that sets the text view’s attributed text to use a serial dispatch queue. Here is my TextViewController:

@IBOutlet private var textView: UITextView?
let q = DispatchQueue(label:"textview")
var htmlText: String? {
    didSet {
        updateTextView()
    }
}
override func viewDidLoad() {
    super.viewDidLoad()
    updateTextView()
}
private func updateTextView() {
    guard self.isViewLoaded else {return}
    guard let s = self.self.htmlText else {return}
    let f = self.textView!.font!
    self.q.async {
        let modifiedFont = String(format:"<span style=\"font-family: '-apple-system', 'HelveticaNeue'; font-size: \(f.pointSize)\">%@</span>", s)
        DispatchQueue.main.async {
            let attrStr = try! NSAttributedString(
                data: modifiedFont.data(using: .unicode, allowLossyConversion: true)!,
                options: [.documentType: NSAttributedString.DocumentType.html],
                documentAttributes: nil)
            self.textView!.attributedText = attrStr
        }
    }
}

Adding print statements reveals that everything happens in the expected order (the order in which htmlText gets set).

matt
  • 515,959
  • 87
  • 875
  • 1,141
  • I’m not saying you couldn’t do the same thing with an extension, but the goal here is to manage the setting of _this_ text view’s attributed text on a serial queue, so it seems best to put everything in the hands of the view controller. – matt Mar 28 '19 at 15:50
  • Great minds! I'd already been experimenting with that approach and agree it does yield the values being set in the right order. However this comment from the docs has me wondering whether I'm just getting away with it (for small input data?): "The HTML importer should not be called from a background thread (that is, the options dictionary includes documentType with a value of html). It will try to synchronize with the main thread, fail, and time out." – Chris Mar 28 '19 at 15:51
  • Okay, if that makes you queasy, then just move the `DispatchQueue.main.async {` up one line to before the `let attrStr`. It _still_ works correctly, and all the attributed string work happens on the main thread. I’ve changed the code to work that way. – matt Mar 28 '19 at 16:04
  • Also I observe you could change my `self.q.async` to `self.q.sync`. – matt Mar 28 '19 at 16:07
  • Good stuff. My version is actually an `OperationQueue` with a `queue.addOperation { ... }`. I'm going to go and read up on whether there's any meaningful difference in those approaches. – Chris Mar 28 '19 at 16:11
0

How about this way to solve the problem?

  private var textView: UITextView? = UITextView()

remove updateTextView() and textView = UITextView() in ViewDidLoad()

E.Coms
  • 11,065
  • 2
  • 23
  • 35
  • Sorry if it wasn't clear, but my code block starting `let theVc = TextViewController()` is already on the main queue - the method it is in is called as a result of a cell select in a `UITableView`. Just to be 100% certain, I tried the additional `DispatchQueue.main.aync { ... }` and it makes no difference. – Chris Mar 28 '19 at 13:26
  • You don't need to call anything in `viewWillAppear`. As long as there is a textView and you set `text` in main thread, it is just working. I repeat your current code and get no problem. So the reason is you must call either `updateTextView` or `setHtmlText ` somewhere in `viewWillAppear`. That's not necessary. – E.Coms Mar 28 '19 at 13:54
  • I suspect you don't have a call to `NSAttributedString(data:options:documentAttributes)` then? That initialiser re-enters the run-loop for the main queue, causing the results to be applied out of order. (see my latest addition to the question) – Chris Mar 28 '19 at 14:18
  • You may init UITextView in the beginning and remove those in viewDidLoad() – E.Coms Mar 28 '19 at 14:49
  • I don't think where it's set matters. Setting it twice in quick succession is what exposes the problem. I'm going to look into avoiding using the HTML initialiser with it's internal run-loop. – Chris Mar 28 '19 at 14:55
  • Here is the case, why do you need to call `before` twice? it may have a bug, if you call it twice. so getting rid of `updateTextView()` is important as you don't need to do it twice. If I call `Before and After`, I did not see "Fetching..." error. But if call "Before, before after after // Before", I have 50% chance to see "Fetching..." – E.Coms Mar 28 '19 at 15:27
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/190855/discussion-between-e-coms-and-chris). – E.Coms Mar 28 '19 at 15:48