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.

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

Previous line is displayed on empty lines in itemview #506

Closed
eyvindbjartiandersen opened this issue May 3, 2019 · 10 comments
Closed

Previous line is displayed on empty lines in itemview #506

eyvindbjartiandersen opened this issue May 3, 2019 · 10 comments
Labels
bug This issue reports a problem that ought to be fixed invalid The issue is based on false assumptions, or is unrelated to Newsboat stfl
Milestone

Comments

@eyvindbjartiandersen
Copy link

eyvindbjartiandersen commented May 3, 2019

newsboat r2.15-83-gdc5c - https://newsboat.org/
Copyright (C) 2006-2015 Andreas Krennmair
Copyright (C) 2015-2019 Alexander Batischev
Copyright (C) 2006-2017 Newsbeuter contributors
Copyright (C) 2017-2019 Newsboat contributors

Newsboat is free software licensed under the MIT License. (Type `newsboat -vv' to see the full text.)
It bundles JSON for Modern C++ library, licensed under the MIT License: https://github.com/nlohmann/json
It bundles an alphanum algorithm implementation licensed under the MIT license: http://www.davekoelle.com/alphanum.html

newsboat r2.15-83-gdc5c
System: Linux 5.0.10-arch1-1-ARCH (x86_64)
Compiler: g++ 8.3.0
ncurses: ncurses 6.1.20180127 (compiled with 6.1)
libcurl: libcurl/7.64.1 OpenSSL/1.1.1b zlib/1.2.11 libidn2/2.1.1 libpsl/0.20.2 (+libidn2/2.1.1) libssh2/1.8.1 nghttp2/1.36.0 (compiled with 7.64.1)
SQLite: 3.28.0 (compiled with 3.27.2)
libxml2: compiled with 2.9.9

Link to video: https://vimeo.com/334141215
I've seen this bug in earlier versions and multiple times with different terminals.

@Minoru
Copy link
Member

Minoru commented May 4, 2019

The link to the video doesn't work for me, Vimeo says "Sorry, we couldn’t find that page". Can you check what could be wrong? Perhaps you need to tick some checkbox to make the video public?

@eyvindbjartiandersen
Copy link
Author

The link should now be working

@Minoru
Copy link
Member

Minoru commented May 4, 2019

Yep, that works; thank you! No idea what's causing this, here's a few questions to get me some more facts:

  • do you have any idea how to reproduce this?
  • can you post your config file?
  • does this happen every time you view an article, or only sometimes?
  • does it happen after resizing the terminal?
  • do you run Newsboat inside Tmux or Screen?
  • what does echo $TERM print out?
  • does pressing CTRL-L fix it?
  • what shell are you using? (In case this is a duplicate of Newsbeuter looks wrong after resizing the terminal akrennmair/newsbeuter#492)

@eyvindbjartiandersen
Copy link
Author

eyvindbjartiandersen commented May 4, 2019

I can reproduce it when scrolling through this feed with this link both in St term and Alacritty: https://eklitzke.org/the-cbc-padding-oracle-problem. The bug appears after you've scrolled past some troublesome text. It happens with only a few feeds and mostly around code or symbols and resizing doesn't seem to affect it and I can reproduce in St with Bash, Sh, but I use Fish . I don't use Tmux or Screen.

  • CTRL-L has no effect.

  • This bug appears in latest version from Git and Arch Linux AUR.

  • echo $TERM yields st-256color.

Config:

history

history-limit 1024
max-items 256
show-keymap-hint no
auto-reload no
prepopulate-query-feeds yes

settings

cleanup-on-quit yes
download-retries 3
download-timeout 30
refresh-on-startup yes
player mpv
browser google-chrome-unstable
reload-threads 8

sorting

article-sort-order date-asc
feed-sort-order date-asc

unbind keys

unbind-key '
unbind-key a
unbind-key q
unbind-key o
unbind-key R
unbind-key e
unbind-key u
unbind-key ;
unbind-key n
unbind-key p
unbind-key i
unbind-key T
unbind-key t
unbind-key ENTER
unbind-key SPACE
unbind-key l
unbind-key L
unbind-key ^N

bind keys

goto-next-feed no
bind-key e down
bind-key E pagedown
bind-key n quit
bind-key i open
bind-key O goto-url
bind-key u up
bind-key t set-tag
bind-key r reload
bind-key R reload-all
bind-key U pageup
bind-key e down tagselection
bind-key u up tagselection
bind-key ENTER open-in-browser
bind-key SPACE toggle-article-read

colors

color background default default
color listnormal default default
color listnormal_unread default default bold
color listfocus default color8
color listfocus_unread default color8 bold
color info default default
color article default default

highlighting

highlight article "^(Feed|Title|Author|Link|Date):." default default
highlight article "^Links:" default default
highlight article "\[[0-9][0-9]
\]" default default
highlight article "\[image [0-9][0-9]\]" default default
highlight article ":.
\(link\)$" default default
highlight article ":.*\(image\)$" default default

title formatting

articlelist-format "%?T?|%-17T| ?%t"
feedlist-format "%t"
feedlist-title-format "%u unread feeds"
searchresult-title-format "Search result %u/%t"
articlelist-title-format "%T"
help-title-format "Help"
selecttag-title-format "Select a tag"
itemview-title-format "%T"
filebrowser-title-format " %?O?Open file:&Save file? - %f"
selectfilter-title-format "Select a filter"
urlview-title-format "Urls found in this article"
dialogs-title-format "Select a dialog"

@Minoru
Copy link
Member

Minoru commented May 4, 2019

Reproduced with a8972c6, https://eklitzke.org/index.rss, and empty config. In my case, the empty lines are replaced by " < i)i++ {". That looks like Newsboat fails to properly escape curly brackets, but I'll have to dig deeper to make sure.

Thank you for the report, @eyvindbjartiandersen!

@Minoru Minoru added the bug This issue reports a problem that ought to be fixed label May 4, 2019
@Minoru Minoru changed the title Possible redraw bug - Previous line is displayed on empty lines Previous line is displayed on empty lines in itemview May 4, 2019
@dennisschagt
Copy link
Member

It looks like this is a bug in stfl.

I've imported the stfl svn repo in git and uploaded to Github so parts of it can be more easily linked to.

Context

stfl_print_richtext(…) is used to print text while applying colors/attributes (based on inline style markers like <blue> and </>).
It gets called in 2 different ways by wt_textview_draw(…) whenever a redraw is needed.

  1. First it gets called for lines which are outside of the viewport. It looks like this is done to interpret and apply style markers so the correct style is applied at the start of the viewport. Argument width == 0 is used to specify that text should not be printed.
  2. Then it gets called for each line in the viewport to actually print text (and still interpret style markers).

Issue

In stfl_print_richtext(…), the code for handling "<>"[a] does not respect the width argument.
Even if width == 0, it prints the '<' character and increments x.
This results in an underflow[b] in the next iteration of the while loop , which in turn results in all remaining text in that line to be printed.

The unintended printing happens on the first line of the textview widget (y == w->y).
In most cases, it will be overwritten by the first line in the viewport but an empty/short first line will not overwrite the characters.

Some extra info

[a] "<>" is an escape sequence for the literal '<' character.
[b] For example, assuming x == 0 => end_col == 0 => end_col - x == underflow
[c] Relevant stfl file

@dennisschagt
Copy link
Member

dennisschagt commented Mar 7, 2020

Following is a patch for stfl which fixes this issue.
I will see if I can find a bugtracker for stfl and otherwise ask for this fix to get applied on their mailing list.

diff --git a/base.c b/base.c
index 9adb254..07c5fbf 100644
--- a/base.c
+++ b/base.c
@@ -750,9 +750,11 @@ unsigned int stfl_print_richtext(struct stfl_widget *w, WINDOW *win, unsigned in
 				wmemcpy(stylename, p1 + 1, p2 - p1 - 1);
 				stylename[p2 - p1 - 1] = L'\0';
 				if (wcscmp(stylename,L"")==0) {
-					mvwaddnwstr(win, y, x, L"<", 1);
-					retval += 1;
-					++x;
+					if (end_col - x > 0) {
+						mvwaddnwstr(win, y, x, L"<", 1);
+						retval += 1;
+						++x;
+					}
 				} else if (wcscmp(stylename, L"/")==0) {
 					stfl_style(win, style_normal);
 				} else {

Edit1: Above patch is available under LGPL 3+ and MIT. Use it however you like.
Edit2: I've contacted the author of STFL on Twitter.
Latest I heard from her is that she would look into it (~1 week ago).
As I haven't heard from her afterwards I just applied the patch in my clone of the svn repo: dennisschagt/stfl#2

@Minoru
Copy link
Member

Minoru commented Mar 9, 2020

Thanks for digging in! STFL's mailing list is pretty dead: I've been on it since December 2015, and never saw any posts by STFL's author.

The next best thing we can do is mention the patch in our changelog. Maintainers in distros will (hopefully) help patch libstfl in their distros. I'll do that on the release day if STFL author doesn't accept your patch before that.

I also think it'd be good to explicitly state that your patch is under LGPL 3+, same as STFL itself, so maintainers don't have to second-guess that.

Since the bug is not with Newsboat itself, I'm closing this as invalid.

@Minoru Minoru closed this as completed Mar 9, 2020
@Minoru Minoru added the invalid The issue is based on false assumptions, or is unrelated to Newsboat label Mar 9, 2020
Minoru added a commit that referenced this issue Mar 9, 2020
@dennisschagt
Copy link
Member

STFL's mailing list is pretty dead: I've been on it since December 2015, and never saw any posts by STFL's author.

I'm wondering if the list works at all as the archive doesn't list any emails.
@Minoru: In case you are still subscribed, did you receive the email I send to that list last Saturday (2020-03-07)?
(subject: Bug report + patch for issue with printing '<' in stfl_print_richtext)

I also think it'd be good to explicitly state that your patch is under LGPL 3+, same as STFL itself, so maintainers don't have to second-guess that.

Done

@Minoru
Copy link
Member

Minoru commented Mar 11, 2020

@dennisschagt Yes, I received that email. Apparently it's only the archive that doesn't work.

carno pushed a commit to carno/newsboat that referenced this issue Mar 20, 2020
@Minoru Minoru added this to the 2.19 milestone Mar 22, 2020
Jehops pushed a commit to Jehops/freebsd-ports-legacy that referenced this issue Mar 29, 2020
This is needed by newsboat to fix
newsboat/newsboat#506

While here also strip the library.

PR:		245001
Approved by:	arved (maintainer)


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@529797 35697150-7ecd-e111-bb59-0022644237b5
uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Mar 29, 2020
This is needed by newsboat to fix
newsboat/newsboat#506

While here also strip the library.

PR:		245001
Approved by:	arved (maintainer)


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@529797 35697150-7ecd-e111-bb59-0022644237b5
uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Mar 29, 2020
This is needed by newsboat to fix
newsboat/newsboat#506

While here also strip the library.

PR:		245001
Approved by:	arved (maintainer)
MarcusCalhoun-Lopez added a commit to macports/macports-ports that referenced this issue May 17, 2020
svmhdvn pushed a commit to svmhdvn/freebsd-ports that referenced this issue Jan 10, 2024
This is needed by newsboat to fix
newsboat/newsboat#506

While here also strip the library.

PR:		245001
Approved by:	arved (maintainer)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue reports a problem that ought to be fixed invalid The issue is based on false assumptions, or is unrelated to Newsboat stfl
Projects
None yet
Development

No branches or pull requests

3 participants