bugGNU nano - Bugs: bug #52258, with softwrap, combining...

 
 

bug #52258: with softwrap, combining characters at end of chunk are not displayed

Submitted by:  Peter Passchier <pepa65>
Submitted on:  Sat 21 Oct 2017 02:15:34 AM UTC  
 
Severity: 3 - NormalStatus: Fixed
Assigned to: David Lawrence Ramsey <dolorous>Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sun 19 Nov 2017 08:26:23 AM UTC, comment #18:

Released in 2.9.0. Thanks for reporting. And fixing.

Benno Schulenberg <bens>
Project Administrator
Wed 25 Oct 2017 05:14:54 PM UTC, comment #17:

The patch looks logical to me. The only thing is: the mbwidth() function is somewhat costly, and it needs to be called only when column == beyond. So... before pushing, I have swapped the two conditions.

Fixed in git, d344c3d0. Thanks for the patch. And thanks for reporting.

Benno Schulenberg <bens>
Project Administrator
Tue 24 Oct 2017 03:38:28 PM UTC, comment #16:

I've tried it with all the problematic lines from the file in comment #1, both with and without softwrap. Adding text until the combining characters are at the edge of the screen works in softwrap mdoe, and scrolling forward and back works when not in softwrap mode. So, in short, everything seems to work.

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Mon 23 Oct 2017 07:26:14 PM UTC, comment #15:

Attached a patch that seems to fix it; I'm still testing it.

(file #42243)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Mon 23 Oct 2017 06:13:00 PM UTC, comment #14:

And after more testing, the problem is not limited to softwrap mode: any line run through display_string() that ends in combining characters has problems (although outside of softwrap mode, they're different problems). For example, in an 80x24 terminal, and current git, run:

nano --ignore --softwrap --nowrap combining-chars2.txt

The line is exactly 80 columns wide, so fits exactly in one row of the terminal. As we've seen, the combining character at the end of the line is not displayed.

Now, press Meta-$ to toggle softwrap mode off. The final character of the line, plus its combining character, both disappear entirely. Then press End, and you'll see that the character and its following combining character are still there, since they are then drawn before the edge of the screen. Finally, press Backspace to delete the combining character, and press Home. The final character of the line will then be displayed (minus the backspaced-over combining character, of course).

So it seems that the real problem is that display_string() has problems displaying combining characters at the final column of the string passed to it. In short:

1. The original softwrap issue is triggered because softwrapping a line before the edge of the screen means that the final column of the string passed to it is the final column of the displayed line, so the problem always shows up if combining characters occur at the end of a softwrap chunk shorter than the screen width.

2. The new non-softwrap issue is triggered because, outside of softwrap mode, display_string() is generally called with a final column of editwincols, so the problem always shows up if combining characters occur at the final column at the edge of the screen.

(file #42242)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Mon 23 Oct 2017 05:32:53 PM UTC, comment #13:

And note that a chunk with missing combining characters due to this bug doesn't actually drop the characters (nano's navigation doesn't skip over them, so trying to move over them reveals them, and you can also tell by the column count that they're there); the characters are in the chunk, but they just aren't displayed. I've updated the bug description to reflect this.

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Mon 23 Oct 2017 05:14:53 PM UTC, comment #12:

And, for the record, nanocrap2.txt is the same file as the one attached to:

https://savannah.gnu.org/bugs/?52183

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Mon 23 Oct 2017 05:13:08 PM UTC, comment #11:

Addendum: Actually, my patch does cause some breakage in certain cases. With it, and current git (4d35835), run:

nano --ignore --nowrap nanocrap2.txt

Press End, and then press Home; the first character of the two-column "^@" will erroneously be on the next line.

So that patch won't work, but it still narrowed down the problem: the column range check in display_string() that uses beyond is cutting the line off too early, before the zero-width combining characters gets displayed.

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Mon 23 Oct 2017 05:04:27 PM UTC, comment #10:

Looking into this, it isn't really the softwrap code per se causing this, it's the display_string() calls that the softwrap code is making to display the strings that's causing it. Since display_string() takes a range of columns to display, and combining characters are zero columns wide, that's leading to their being cut off (and only on lines that take up one row or less).

The attached experimental patch seems to fix it. Given the comments in display_string() regarding column and span, this shouldn't work, but it does in my preliminary testing so far, so maybe the comments are wrong somewhere? (The fact that, according to comments, column is one-based while span is zero-based makes this rather confusing. Or maybe at least one of those two assumptions is not actually true?)

(file #42238)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Mon 23 Oct 2017 04:01:45 PM UTC, comment #9:

Okay! Can reproduce now. Getting the author of the softwrap code into the loop... CC'ed.

Benno Schulenberg <bens>
Project Administrator
Mon 23 Oct 2017 02:30:43 AM UTC, comment #8:

It's a bug with softwrap. If I turn it off, all is normal.

Peter Passchier <pepa65>
Sun 22 Oct 2017 04:59:05 PM UTC, comment #7:

So, if you take the 2.8.5 tarball and compile that, it renders the UTF testing text fine? Do you use softwrapping? And if you run the last released version (2.8.7) with --ignorercfiles, does it maybe render fine? How wide are the terminals that you use?

[Trying to reproduce this bug, I've run into an absurd rendering bug on Linux Mint 18 Sarah (which is based on Ubuntu 16.04, Xenial, if I'm not mistaken). Will report that shortly, although I don't think it's a nano bug.]

Benno Schulenberg <bens>
Project Administrator
Sun 22 Oct 2017 02:43:01 PM UTC, comment #6:

The v3.5.3 branch is working OK here.

I've never done git bisect, but to rebuild each time I did:
./autogen.sh
./configure
make
src/nano ~/UTF-8-demo.txt

And then marked the bisect as good or bad. The end result was:

Bisecting: 0 revisions left to test after this (roughly 0 steps)
[8490f4acab53ed38c3b083871654a073e293e02c] softwrap: make the changes to actually allow the chunk width to vary

Peter Passchier <pepa65>
Sun 22 Oct 2017 06:48:51 AM UTC, comment #5:

If you do a 'git checkout v2.5.3' and build that, does that "suppress" some of the vectors and tone marks?

If yes, then it is a build problem. If no, then please do a 'git bisect' to find the commit that broke this.

Benno Schulenberg <bens>
Project Administrator
Sat 21 Oct 2017 02:44:03 PM UTC, comment #4:

Well, that makes it non-reproducable I guess... Still wonder why that happens here only with nano.

I installed nano from the package manager:
GNU nano, version 2.5.3
(C) 1999..2016 Free Software Foundation, Inc.
Email: -unavailable- Web: http://www.nano-editor.org/
Compiled options: --disable-libmagic --disable-wrapping-as-root --enable-utf8

And this version is fine. Is it something in the build somehow?

Peter Passchier <pepa65>
Sat 21 Oct 2017 09:36:51 AM UTC, comment #3:

Yes, the vector sign is on the b like it is on the a in line 57, and the tone mark at the end of line 123 is the same as the mark in the middle of the first half of line 127, and the mark at the end of line 129 is the same as the one at the start of line 130. They are all there. Visible and deletable and restorable.

Benno Schulenberg <bens>
Project Administrator
Sat 21 Oct 2017 08:44:26 AM UTC, comment #2:

Hmm, then it sounds like I might have a local problem..? The vector sign on the b is missing in line 57, and the tone marks on the last phonemes of lines 123 and 129. Just to check: those are all present on your system?

I just pulled nano from the github master branch and did:
./configure
make
sudo make install

Terminals: Sakura, ROXTerm, MATE Terminal
Locale en_US.UTF-8
Distro: Ubuntu MATE 16.04.3

Peter Passchier <pepa65>
Sat 21 Oct 2017 08:19:13 AM UTC, comment #1:

I don't see any differences between how nano renders those lines and how less and vim do.

Which terminal are you using? Which locale? What distro and version?

For me: xfce4-terminal 0.8.4 (Xfce 4.12), nl_NL.UTF-8, Xubuntu 17.04.

Benno Schulenberg <bens>
Project Administrator
Sat 21 Oct 2017 02:15:34 AM UTC, original submission:

Comparing nano's rendering with cat and vi of http://www.cl.cam.ac.uk/%7Emgk25/ucs/examples/UTF-8-demo.txt
exposed little rendering errors in lines 57, 123, 129.

GNU nano from git, v2.8.7-56-gca8317e
(C) 1999-2011, 2013-2017 Free Software Foundation, Inc.
(C) 2014-2017 the contributors to nano
Email: -unavailable- Web: https://nano-editor.org/
Compiled options: --disable-libmagic --enable-utf8

Peter Passchier <pepa65>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by dolorous (Updated the item)
  • -unavailable- added by bens
  • -unavailable- added by bens (Posted a comment)
  • -unavailable- added by pepa65 (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 19 Nov 2017 08:26:23 AM UTCbensOpen/ClosedOpen=>Closed
    Wed 25 Oct 2017 05:14:54 PM UTCbensStatusConfirmed=>Fixed
      Assigned toNone=>dolorous
    Mon 23 Oct 2017 07:26:14 PM UTCdolorousAttached File-=>Added 0001-display-don-t-cut-off-zero-width-characters-at-the-e.patch, #42243
    Mon 23 Oct 2017 06:13:00 PM UTCdolorousAttached File-=>Added combining-chars2.txt, #42242
    Mon 23 Oct 2017 05:32:53 PM UTCdolorousSummarywith softwrap, combining characters at end of chunk are dropped=>with softwrap, combining characters at end of chunk are not displayed
    Mon 23 Oct 2017 05:04:27 PM UTCdolorousAttached File-=>Added off-by-one-display-string.patch, #42238
    Mon 23 Oct 2017 04:01:45 PM UTCbensStatusNeed Info=>Confirmed
      SummaryUnicode rendering bugs=>with softwrap, combining characters at end of chunk are dropped
      Carbon-Copy-=>Added dolorous
    Sat 21 Oct 2017 08:19:13 AM UTCbensStatusNone=>Need Info

    Back to the top


    Powered by Savane 3.1-cleanup1