missing or duplicate lines caused by a wrapped line with wide characters

[Toshio Ito]

[Impact]

Currently less doesn't work correctly with UTF-8 encoded Japanese characters, wrapping the lines in an invalid manner, missing or duplicating them (see original description below for exact error cases). For locale like these it breaks the general workflow, making the tool unreliable.

[Test Case]

1. Open xterm.
2. Set the geometory of xterm to 71x22.
3. Open the attached lesstest.long_jap.txt with less (maybe you need
   environment variable LANG=ja_JP.UTF-8)
4. Type "j", then you will see "003" at the top and only the first part of
   the wrapped line should be shown.
5. Type "k", then you will see "001", "002" and "003" at the top.

[Regression Potential]

Rather low as the fix is present in the beta version of less for over 5 months already, which seems to be enough time for general audience testing. But since the width tables are modified potentially this could lead to other similar breakages related to wide-character handling.

[Original Description]

When you scroll down text with "j" key and encounter a wrapped line
with wide characters (such as UTF8-encoded Japanese characters),
"less" seems to show the whole wrapped line at a single "j" key,
causing the view scroll down by 2 lines at once. Strangely, if you
type "k" to scroll up, it does that by only 1 line. As a result, there
is a missing line that should have been shown.

Even stranger stuff (i.e., duplicate lines) happens when you type "j"
and "k" alternately when a wrapped line with wide characters is at the
bottom of the view.

# Steps to reproduce

1. Open xterm.
2. Set the geometory of xterm to 71x22.
3. Open the attached lesstest.long_jap.txt with less (maybe you need
   environment variable LANG=ja_JP.UTF-8)
4. Type "j", then you will see "003" at the top, and a long wrapped
   line with Japanese characters at the bottom.
5. Type "k", then you will see "001" and "003" at the top. "002" is
   missing.

# Expected behavior

In step 4, only the first part of the wrapped line should be shown.

In step 5, all "001", "002" and "003" should be shown.

# Test Environment

- Xubuntu 16.04 (Xenial) beta (in VirtualBox on Xubuntu 14.04)
- less: 481-2.1
- xterm: 322-1ubuntu1
- Japanese environment (LANG=ja_JP.UTF-8)

# Note

- The same problem happens on xfce4-terminal (0.6.3-2ubuntu1)
- lv (4.51-2.3build1) doesn't have such problem.
- In "less" in Xubuntu 14.04 (version 458-2), this problem didn't
  exist.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[aeb (aeb-cwi)]

I just investigated the same bug. "less" loses input lines because it does not count output lines correctly. The problem is that it assumes that east asian wide characters have width 1, but in reality, e.g. on xterm, these characters take 2 positions. In other words, the routine pwidth() in less-481:line.c is broken (since the is_wide_char(ch) returns false for characters that take two screen positions).

[Mitsuya Shibata (cosmos-door)]

The table of width of characters in less-481 is incorrect. I fixed table generator script and re-generate table files.

Because of branch of bazaar is old (wily?), I attached debdiff file instead of merge request. This patch is already forwarded to upstream.

Please sponsor it.

[Brian Murray (brian-murray)]

Where was this fixed upstream? Could you provide a link to the fix being accepted upstream?

[Mitsuya Shibata (cosmos-door)]

Hi Brian,

Fixed in less-487, unfortunately it is in BETA yet.

http://www.greenwoodsoftware.com/less/
http://www.greenwoodsoftware.com/less/news.487.html

> Fix bug in Unicode handling that missed some double width characters.

[Łukasz Zemczak (sil2100)]

Hello Mitsuya! Thank you for preparing the debdiff. I checked upstream and the bug in mention was indeed fixed there - but it seems that their fix for mkutable (at upstream) is different. I would prefer if Ubuntu didn't diverge from upstream if possible. Is it because the way it's done in 487 is not applicable on top of 481? Or what is the reason for the differences?

Thanks!

[Mitsuya Shibata (cosmos-door)]

Hi Łukasz!

Diffs mkutable is just origin of author (between me and upstream author).
It seems that mkutable in 487 is more fine.
I attach new debdiff based on 487. Please review it.

Anyway, normal build process doesn't use mkutable.
mkutable script is only used by Makefile.aut when generating source archive for new version.
More important components are FOO.uni files.
There are generated by mkutable and included source archive.

Tanks!

[Mitsuya Shibata (cosmos-door)]

I would like to SRU to xenial and yakkety.
Could you set affected distribution?

[Łukasz Zemczak (sil2100)]

Hello Mitsuya! Thank you for the modified debdiff, I have sponsored your change to the ubuntu archives - the package is currently in the zesty UNAPPROVED queue as we're in freeze. It should appear in the archives once some release manager reviews it.

Once approved, I can also sponsor it for xenial and yakkety. I personally think that the bug can be annoying enough to be considered for an SRU (missing lines in less is never a good thing).

[Łukasz Zemczak (sil2100)]

I just sponsored this to xenial and yakkety. If you have a moment please check if the SRU-enabled bug description and test case are good enough.

[Launchpad Janitor (janitor)]

This bug was fixed in the package less - 481-2.1ubuntu2

---------------
less (481-2.1ubuntu2) zesty; urgency=medium

  * debian/patches/lp-1562308-fix_mkutable.patch:
    - Fix east asian width problem (LP: #1562308)

 -- Mitsuya Shibata <email address hidden> Sat, 25 Mar 2017 03:00:03 +0900

[Chris J Arges (arges)]

Hello Toshio, or anyone else affected,

Accepted less into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/less/481-2.1ubuntu0.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Chris J Arges (arges)]

Hello Toshio, or anyone else affected,

Accepted less into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/less/481-2.1ubuntu1.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Mitsuya Shibata (cosmos-door)]

Hi Łukasz and Chris,

Thank you for SRUing!

I done testing new version both on xenial and yakkety without any problem.

[G.M. (sexxxenator)]

Hi,

I think I have the same bug.

What happens is the following:

---- Example file
Apr 10, 2017 10:46:33 AM fr.presans.jsofia.domain.DReview cleanTitle
FINER: >>> AFTER QUOTE=realtime hand tracking using modificated flocks of features algorithm realtime hand tracking using modificated flocks of fea tures algorithm information sciences and technologies bulletin of the slovakia special section on student research in inf
Apr 10, 2017 10:46:33 AM fr.presans.jsofia.domain.DReview cleanTitle
FINER: >>> AFTER ACM/IEEE=realtime hand tracking using modificated flocks of features algorithm realtime hand tracking using modificated flocks of fea tures algorithm information sciences and technologies bulletin of the slovakia special section on student research in inf
Apr 10, 2017 10:46:33 AM fr.presans.jsofia.domain.DReview cleanTitle
FINER: >>> AFTER PAGE/VOL=realtime hand tracking using modificated flocks of features algorithm realtime hand tracking using modificated flocks of fea tures algorithm information sciences and technologies bulletin of the slovakia special section on student research in inf
Apr 10, 2017 10:46:33 AM fr.presans.jsofia.domain.DReview cleanTitle
FINER: >>> AFTER EDs=realtime hand tracking using modificated flocks of features algorithm realtime hand tracking using modificated flocks of fea tures algorithm information sciences and technologies bulletin of the slovakia special section on student research in inf
Apr 10, 2017 10:46:33 AM fr.presans.jsofia.domain.DReview cleanTitle
FINER: >>> AFTER PARENTHESIS=realtime hand tracking using modificated flocks of features algorithm realtime hand tracking using modificated flocks of fea tures algorithm information sciences and technologies bulletin of the slovakia special section on student research in inf
Apr 10, 2017 10:46:33 AM fr.presans.jsofia.domain.DReview cleanTitle
FINER: >>> AFTER SPACING CLEAN=realtime hand tracking using modificated flocks of features algorithm realtime hand tracking using modificated flocks of fea tures algorithm information sciences and technologies bulletin of the slovakia special section on student research in inf
Apr 10, 2017 10:46:33 AM fr.presans.jsofia.domain.DReview titleMatcher
FINER: Found NON matching chars: a≠r
[+ a lot of spaces at the end]
-----

Opening this file with less, the ngoing up & down with arrows or pgup/pgdown, you'll notice that the lines with '>>>' will merge into a single line.

See http://imgur.com/a/7ol45

I'm using:
$ uname -a
Linux gyom 4.10.0-15-generic #17-Ubuntu SMP Fri Mar 24 17:51:38 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

$ cat /etc/issue
Ubuntu Zesty Zapus (development branch) \n \l

$ less --version
less 481 (GNU regular expressions)
Copyright (C) 1984-2015 Mark Nudelman

I'll try the new package right now

[G.M. (sexxxenator)]

OK... Just tried to test the new package....

# apt-get install less
Reading package lists... Done
Building dependency tree
Reading state information... Done
less is already the newest version (481-2.1ubuntu2).

But the problem is still there, so the patch is unsufficient....

[Mitsuya Shibata (cosmos-door)]

Hi, G.M.

> Opening this file with less, the ngoing up & down with arrows or pgup/pgdown, you'll notice that the lines with '>>>' will merge into a single line.

I cannot reproduce it on gnome-terminal 70x24.

1. Could you upload your reproducible file?

2. What about terminal software and its window size?

3. If use "less -N file", what it will be?

Thanks,

[Brian Murray (brian-murray)]

I believe the text in comment #17 can be used to create the file to reproduce the bug, that being said I didn't see any issues with it. Regardless, I'm going to tag this v-failed so that it doesn't accidentally get released until comment #17 is investigated further.

[Mitsuya Shibata (cosmos-door)]

Hi Brian and G.M.,

> until comment #17 is investigated further.

IMHO, comment #17 is other bug and not affect this patch.

* The original bug is reproducible by description way.
* Already confirmed that the original bag is fixed with this patch.
* comment #17 isn't reproducible at least on Brian's and my environment.
* Not known whether comment #17 is reproducible with old version.

Additionally to this, the original bug and patch is mainly affect
wrapped line with wide characters.
However sample text on comment #17 has a line with wide character, and
it seems that the line isn't wrapped why is very short.

> G.M.

Could you explain how to reproducible method and upload reproducible file?
Is it can reproduce on 16.04 or 16.10?

If not, could you filed your problem as other bug?

Thanks,

[Mitsuya Shibata (cosmos-door)]

Hi Brian and G.M.,

>> until comment #17 is investigated further.
>
> IMHO, comment #17 is other bug and not affect this patch.
>
> Is it can reproduce on 16.04 or 16.10?
>
> If not, could you filed your problem as other bug?

Is there any update?

Thanks,

[Mitsuya Shibata (cosmos-door)]

no response about failure. removed "verification-failed".

[Robie Basak (racb)]

10:29 <rbasak> bdmurray: opinion on bug 1562308 please? It seems to me that comment 17 is referring to some other bug, with no steps to reproduce provided. OTOH cosmos-door has a reproducer and confirmed it broken without and fixed with proposed, so I think this is OK to release. Do you agree?

6:07 <bdmurray> rbasak: I looked for other bugs related to the commenter and found some recent activity from them so maybe they made a mistake? Also I'm looking at other less bugs to see if there is anything like their issue.

16:10 <bdmurray> rbasak: I don't see anything similar to comment #17 so I think its fine.

I suspect sexxxenator has a different bug, so I'm releasing this one. sexxxenator, please find or file a different bug for your issue.

[Robie Basak (racb)]

The verification of the Stable Release Update for less has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package less - 481-2.1ubuntu1.1

---------------
less (481-2.1ubuntu1.1) yakkety; urgency=medium

  * debian/patches/lp-1562308-fix_mkutable.patch:
    - Fix east asian width problem (LP: #1562308)

 -- Mitsuya Shibata <email address hidden> Sat, 25 Mar 2017 03:00:03 +0900

[Launchpad Janitor (janitor)]

This bug was fixed in the package less - 481-2.1ubuntu0.2

---------------
less (481-2.1ubuntu0.2) xenial; urgency=medium

  * debian/patches/lp-1562308-fix_mkutable.patch:
    - Fix east asian width problem (LP: #1562308)

 -- Mitsuya Shibata <email address hidden> Sat, 25 Mar 2017 03:00:03 +0900

[Mitsuya Shibata (cosmos-door)]

> bdmurray

Thanks, uploading!