GNOME Bugzilla – Bug 754596
Fix bce madness
Last modified: 2017-05-04 09:46:27 UTC
Created attachment 310689 [details] Screenshot This has been bugging me forever... The bce (background color erase) terminal feature is broken by design and often causes undesired result. Consider a simple utility that prints stuff, occasionally with a nondefault background. If a line happens to wrap with such a nondefault background, the entire next line is painted with that color for no sane reason, making it look terrible and definitely not the way intended. But, er... wait... it only happens if the printed line makes the terminal scroll (that is, usually not for the first 23 lines of the lifetime of the terminal), so it makes even less sense. See the screenshot where an "ls -l /dev" was issued at that window size and scrolled back to almost the top. The top half looks as desired: text with back blackground wraps to the next line but other than that it's nice and readable. The bottom half with the black background filling up entire rows just does not make any sense. This is also probably the only point where rewrap on resize can't do a reasonable job, you sure won't end up with the same look than as if you ran "ls -l /dev" at a different window size and resized the window later. Even worse: grep, as a workaround, emits a \e[K after switching back to the default color, causing a repaint of the rest of the line with the default color. This introduces an even more severe bug: it can strip off a character at the end of the line, see bug 740789 and http://savannah.gnu.org/bugs/?36831 (let alone a possible, albeit unlikely flicker). In turn, vte adds another workaround intentionally deviating for xterm so that grep doesn't eat a character in gnome-terminal, although it does in xterm and probably some other emulators. bce is a nice feature for fullscreen apps: it optimizes on data to be transmitted (honestly: who cares nowadays?) and hypothetically allows the whitespaces at the end of the line not to get copy-pasted (but ncurses probably never really cares about it and probably neither do users, since copy-pasting across line boundary kinda only makes sense with non-ncurses utilities' outputs). Still, bce is a nice feature if the erase is an explicit operation initiated by an escape sequence (such as \e[K). But it's braindamaged to have it applied implicitly when a new terminal line happens to scroll in at the bottom of the screen. Is there anything that could be done to end this madness (not just for gnome-terminal but all the terminals out there) once and for all? I talked to Thomas (xterm+ncurses) about a year ago and he wasn't quite interested (kinda had the opinion that any change would break existing apps). For starter, we could just "fix" our code (remove the "Match xterm and fill the new row when scrolling" 6 lines from _vte_terminal_cursor_down()) for the next development cycle and see if anyone complains about something breaking. Then we'll have a better understanding of the situation. It's tricky because it might require buy-in from some key people (e.g. Thomas, making sure that ncurses doesn't rely on the bce flag for the newly appeared row's colors, or to introduce a new flag that describes the new behavior), and perhaps wait for a couple of years to get it deployed across most systems...??? Any opinions?
Someone mentioning the same problem: http://stackoverflow.com/questions/32769876/sgr-reset-code-is-ignored-if-n-is-placed-on-a-new-line
(In reply to Egmont Koblinger from comment #0) > For starter, we could just "fix" our code (remove the "Match xterm and fill > the new row when scrolling" 6 lines from _vte_terminal_cursor_down()) for > the next development cycle and see if anyone complains about something > breaking. Then we'll have a better understanding of the situation. Let's try that :-)
> Let's try that :-) Submitted as 13b7476. /me grabs coke & popcorn, waiting for bugreports flying in :) I suggest to leave this bugreport open until we "finalize" this feature in the next stable series.
Just occurred to me: ~2 years ago we fixed relevant bug 710486 which didn't have any duplicate reports and probably went undiscovered for many years. This gives hope!
Hi all, Arrived here because the bce / screen scroll issue didn't make any sense to me either and I'd rather it work as proposed in this bug. In fact, it seems to work exactly as proposed here inside of Tmux. It was the difference between running with and without Tmux that made me think about it at all. Anyway, it seems that unfortunately some applications actually do depend on things running with the existing behavior. Check out Thomas Dickey's comment here: http://unix.stackexchange.com/questions/212933/background-color-whitespace-when-end-of-the-terminal-reached His post actually linked to this bug report and is how I found it. Anyway, as the author of Xterm, his experience should carry some weight. Anyway, after having spent a few minutes thinking about this, it seems that regardless of whatever decision VTE makes, applications should protect against this behavior by turning off background color before generating a newline. Unless of course they rely on the current behavior.
Will throw this out there for whatever it's worth. It seems it's much easier for applications to work around the existing behavior, than it is to recover this behavior in other situations. For instance, if i'm in a VTE session and what to mark it as different than the others, I can just do "tput setab 3 ; clear" and it will maintain that color properly, including when I scroll. With the proposed new behavior that use case no longer works since when I scroll the background color that I have selected will not be used. On the other hand, any application that wants to ensure that the background color doesn't "leak" when scrolling, needs only disable background colors before each newline. This proposed change will break some things that are currently possible. AFAIK, after this proposed change there will be no escape sequence to effectively set the default colors. And the default colors will now always be used when scrolling. So while I initially totally supported this proposed change, it now seems like a mistake.
Thanks for this really valuable input! I've no time right now to process this and understand what we exactly broke, but will definitely get back to it. Just wondering... one more thing we could do is respect bce if an explicit newline is printed by the app, but disregard bce on an implicit overflow to the next line. I'll really need to see if it improves the situation! Note: If we break _some_ utilities (that is, only a few special rare ones) then I personally wouldn't mind the change and wouldn't revert – we'd still fix more than that and have a more reasonable behavior. The problem with xterm's bce approach is that it's outright impossible for utilities to achieve a certain typical sane behavior. With our new approach we might break some utilities, but those might easily be adjusted to fix the breakage. That being said, if it turns out that we broke _many_ apps, that'll be a different story.
(If I were to design this feature from the beginning, what would make sense on an explicit newline is to paint the rest of the _preceding_ line with the color, not the _next_ one as bce does :))
(In reply to Sean Estabrooks from comment #6) > For instance, if i'm in a VTE session and what to mark it as different than > the others, I can just do "tput setab 3 ; clear" and it will maintain that > color properly, including when I scroll. This would break on the first explicit background color change (e.g. "ls -l" on a broken symlink typically) or the first "lazy" revert of attributes (just "\e[m" to revert all attrs to the default, rather than reverting fg color, boldness, etc. separately), and also on the first wide line implicitly overflowing (given the official xterm-like bce behavior). So this use case is invalid, and shouldn't matter for our bce discussions. If you wanted to mark a terminal different, you should switch profile, or switch the default background color using OSC 11 (e.g. xtermcontrol --bg ...).
That's very interesting. The current behavior is a deeper problem than I appreciated. Implicit wrapping is a thorny problem because the proper thing to do depends on the *intent* of the current background color. If the current background color is only being used to highlight an isolated string, then implicit wrap should not paint the new line with it. However, if it is being used to mean, "this is the color i want for my background and the current string is not highlighted, it's regular text" then the new line should be painted with this color. The real problem is that these two meanings are hidden behind a single escape sequence. We don't know if the application wants to highlight just this specific string with a certain background color, or if this color is meant as the default for all empty cells. In an ideal world, there would be an escape sequence that said... use this color as the _DEFAULT_BACKGROUND_COLOR_. The existing escape sequences would only set the CURRENT_BACKGROUND_COLOR. Every new line would initially be painted with the DEFAULT, never with the CURRENT. All BCE operations would always use the DEFAULT_BACKGROUND_COLOR and never be confused by the color of any string currently being printed. That's something that could be implemented in order to appease applications that are affected by the new behavior.
(In reply to Egmont Koblinger from comment #9) > If you wanted to mark a terminal different, you should switch profile, or > switch the default background color using OSC 11 (e.g. xtermcontrol --bg > ...). Right, OSC 11 fits the bill. There might legacy apps that expect the old behavior, but it seems like it should be manageable. This begs the question, why support BCE at all, if the proper way to set the background color is with OCS 11. Maybe the proper thing to do is just disable BCE like Tmux does? Just wondering out loud, haven't thought that through.
(In reply to Sean Estabrooks from comment #10) > That's very interesting. The current behavior is a deeper problem than I > appreciated. Implicit wrapping is a thorny problem because the proper thing > to do depends on the *intent* of the current background color. Yup. > If the current background color is only being used to highlight an isolated > string, then implicit wrap should not paint the new line with it. Yup. With xterm's bce behavior there's no way for a utility to properly workaround and have the desired result. Hence the change in vte. > However, if it is being used to mean, "this is the color i want for my > background and the current string is not highlighted, it's regular text" > then the new line should be painted with this color. This can easily be worked around with the new vte behavior: After outputting the string, but before printing the final newline, print a \e[K (clear to end of line using the current background color). Also, this is a way less typical use case. I can only think of fullscreen apps with this intent, and those work correctly anyways. I can't think of any simple utility (that just prints a stream of text and doesn't do fullscreen terminal emulation) that wants to have a custom background color for an entire paragraph of text. > In an ideal world, there would be an escape sequence that said... Yup, but we don't live in an ideal world :) and such heavy change is not feasible. (In reply to Sean Estabrooks from comment #11) > This begs the question, why support BCE at all, if the proper way to set the > background color is with OCS 11. These are completely different. BCE just (mis)uses the cursor's current background color for a few cells it's about to paint. OSC 11 takes effect on the entire terminal screen (including its scrollback history), pretty much as if you changed the color palette in Preferences. > Maybe the proper thing to do is just > disable BCE like Tmux does? This becomes more complicated here... :) For some reason we use the "xterm" terminal descriptor which does have bce. In order to completely turn it off, we'd need to default to TERM=screen instead (or something vte-specific, which would take about a decade until gets deployed on all machines out there, so it's not feasible). So, why do all graphical emulators use xterm or something alike (with bce), and not screen (which one doesn't have bce)? I don't know. :) But if we changed, we'd need to change probably many other things too (e.g. keycodes), and I'm not sure how many things we'd break. We essentially try to emulate xterm as close as reasonable (meaning: there are exceptions where we think it makes sense, there are tons of exotic features in xterm that we lack etc., but xterm is our reference, not screen/tmux).
So what's the conclusion here: revert for 0.44, or keep?
The change has been submitted for 4+ months, and I haven't seen any bugreports yet (I keep an eye on RH/Ubuntu bugs as well). I'd say let's keep it. Worst case: we'll revert for 0.44.x or 0.46.
Houston... We broke the "le" text editor. Just open any file, and scroll down with the Down arrow. It uses ncursesw, so unfortunately chances are that other ncurses-based apps are broken too. It switches to alternate screen (no surprise from ncurses), and sets up restricted scrolling for the first N-1 lines (e.g. "\e[1;23r") while scrolling the contents upwards. What could we do? 1. Try to convince Thomas to modify ncurses to explicitly set the background color of those lines 2. Disable this new behavior on the alternate screen 3. Disable this new behavior if restricted scrolling is in effect 4. Revert this new behavior entirely (I wouldn't like to, since it fixes many use cases)
A similar patch was tried once for linux console and was reverted (by my request). Changing terminal semantics would indeed break some applications. Fixing coloured ls can be done by \e[m\e[K at the end of line.
> 2. Disable this new behavior on the alternate screen looks feasible, since all full-screen applications (which move cursor) are required to use smcup and it switches to the alternate screen. linux console does not have an alternate screen so it was not an option.
(In reply to Alexander V. Lukyanov from comment #16) > Changing terminal semantics would indeed break some applications. Yup, but those bugs _can_ be fixed. Sticking to the legacy broken behavior results in other bugs that can _not_ be fixed/workedaround. > Fixing coloured ls can be done by \e[m\e[K at the end of line. As mentioned in the opening comment, fixing ls can _not_ be done with \e[K without introducing even more severe problems. This is the real problem here. Otherwise I'd be kinda happy with the old behavior + \e[K.
Almost forgot I already had this idea in comment 7: 5. Legacy behavior on expicit newline, new behavior on implicit overflow to the next line.
(In reply to Egmont Koblinger from comment #19) > 5. Legacy behavior on expicit newline, new behavior on implicit overflow to > the next line. Nice one, I really like it. But I think it would be even better if vte kept track of current line overflow status (reset on a newline or other explicit cursor movement), and erased to EOL with background color on a newline when the overflow status is set. This way if an application actually wants new lines to be filled with background colour, then the new line created by the overflow would be filled too. In other words, if scrolling by newline fills new line with background colour, then the previous line created by overflow should also be filled. The bg colour should be saved at the time of overflow.
Someone else also reports problems: https://bugs.launchpad.net/ubuntu/+source/gnome-terminal/+bug/1678271
Created attachment 350209 [details] [review] bce followup fix Out of the ideas presented above, this simple patch implements probably (hopefully) the simplest and the best. bce is respected and the new line is filled with the current background color if scrolling occurs because of an explicit escape sequence, but is still disregarded when the new line appears due to implicit autowrapping at the end of a line. This fixes the "le" text editor, and keeps the originally reported scenario (see e.g. screenshot) fixed. Let's see if we've fixed Boris's case too in the Ubuntu bugreport.
Comment on attachment 350209 [details] [review] bce followup fix Let's try this for master.
Comment on attachment 350209 [details] [review] bce followup fix Committed to master.
The original risky patch has been submitted for 1.5 years. Since then, we've received two bugreports: the le editor which is now fixed, and dosbox (in the Ubuntu bug) which cannot be reproduced (and is also probably fixed by this followup patch). As such, let me be brave enough to close this bug.
As found at https://superuser.com/questions/1205861/gnome-terminal-vim-colors-not-updating this issue also effects vim (with nondefault background). I've verified, and the recent commit a26a60b fixes this too. I'm wondering how come that a breakage in such a popular app went unnoticed for so long... anyway... it's fixed now. Maybe backport to 0-48?
Fine with me.
Okay, backported.