Newsboat: Revisit FAQ item on how to spawn Firefox

[Minoru]

821 updated an explanation on how to properly spawn Firefox (and other GUI browsers). That PR got merged, but later on @q3cpma found a way to simplify it. Since I don't quite get why the simplified version works, I reverted #821 (https://github.com/newsboat/newsboat/commit/321cd9edc3ae0d09b7b25c7cefa38b10936ceae1). I'm currently busy pushing out 2.19 release, so I'll revisit this issue afterwards.

[Minoru]

855 is kinda related.

[Minoru]

Interestingly, we've been over this a few years back, in Newsbeuter: https://github.com/akrennmair/newsbeuter/issues/597

[Minoru]

This turned out to be a bi-i-ig can of worms! Here's a timeline of the events that led to this issue:

• November 23, 2016 (that's still Newsbeuter days): I replace direct calls to system() with a little helper called utils::run_interactively(). The original calls didn't have any error-handling. My helper contains a feeble attempt at it: I check that the command did start, but incorrectly check its exit status. system() returns a couple values packed into a single int. I was supposed to use macros like WEXITSTATUS to extract the lowest 8 bits of the exit code, but I interpreted the whole return value as the command's exit code.

  At the time, we had no use for exit codes; they were ignored, so the problem didn't affect any users. We also didn't have any reviews. I didn't write any tests for that helper either.

• August 20, 2017: @Arlon1 adds a check for browser's exit code. Neither him nor me find any problems during manual tests, and that code gets merged.

• September 3, 2017: @epon93 reports that Newsbeuter doesn't start the browser for him. Bisect points at Arlon1's PR, and I revert that. I'm confused and don't understand why it behaves that way; I apparently fail to even conceive that utils::run_interactively() might be the culprit. The issue goes onto the back-burner.

• November 13, 2019 (that's Newsboat days now): @mjsir911 re-implements the feature. Neither of us spots the problem while testing, and I don't remember the trouble we had in Newsbeuter, so I merge the PR.

• December 22, 2019: Newsboat 2.18 is released, complete with the bug.

• March 6, 2020: @cozed-up reports that Newsboat says the browser returned error code 32512 (I also remember Hund mentioning this on IRC, although that might've been later on). I still fail to conceive that this might be an actual bug; my then-current belief was that checking for exit codes exposed user's somewhat hacky browser invocations, and I need to figure out the way to fix them. This leads to wild concoctions involving additional shells, nohup, disown and so forth.

  On that same day, @rnestler re-implements utils::run_interactively() in Rust. The new implementation doesn't directly invoke system(), so both of us miss the fact that the old implementation was buggy. This change has two tests, one of them returning 0 and another returning 1. I merge it on March 18th.

• March 20, 2020: I add a new FAQ item explaining how to "correctly" spawn a browser from Newsboat. Within a day, @q3cpma points out some over-engineering, which I can't explain, so I revert the PR and file this issue as a reminder for myself to go back and re-evaluate.

• March 22, 2020: Newsboat 2.19 is released, without the bug — but I don't notice its absence yet.

• April 1, 2020: @tsipinakis files a somewhat related issue. It mentions that Newsboat now takes only the lowest 8 bits of the exit code into account. I make a connection, linking that issue here, but still don't fully understand the situation.

• June 17, 2020, i.e. today: I finally dive into both #855 and this issue, grokking the story behind this at last.

BTW, timelines like this are the reason I insist on keeping discussions in the bug tracker, not IRC or private emails, and might even ask you to write a more descriptive commit message or factor the Git history better. Having all that history at my disposal was invaluable in understanding what's going on. Without it, this issue might've ended up remaining a mystery, or at least would have required way more effort to investigate. Thank you to everyone who takes the time to compose a comment here instead of just pinging me on IRC.

Anyway, it's time for actionables and take-aways:

1. Newsboat 2.18 contains a bug that might prevent users from opening the browser. I'm going to submit a PR with an FAQ item explaining how to wrap the browser in a script that always returns 0. That will neutralize the bug, but will also nullify the feature, effectively restoring pre-2.18 functionality.

2. utils::run_interactively() (and other functions that return exit codes) should return u8 to more clearly indicate that this is not the whole exit code. I'll submit a PR for that as well.

3. This whole story represents an outstanding string of errors on my part: I failed to implement error handling correctly, I then failed to test Arlon1's PR properly, then failed to remember about it when mjsir911 brought the idea back, then also failed to test mjsir911's PR, then failed to realize that buggy utils::run_interactively() can be a root cause for issues. I can only hope that I'm not always that sloppy.

   I also don't see any solution to this. All I can do is lower the possibility of this repeating: I can read manpages more carefully, I can be more thorough with manual testing, I can learn to assume that everything inside Newsboat is broken. Live and learn, I guess.