STR #3361: Text_Display with wrap on hides last line

STR #3361

Application:	FLTK Library
Status:	1 - Closed w/Resolution
Priority:	4 - High, e.g. key functionality not working
Scope:	3 - Applies to all machines and operating systems
Subsystem:	Core Library
Summary:	Text_Display with wrap on hides last line
Version:	1.3.4
Created By:	cleanrock
Assigned To:	greg.ercolano
Fix Version:	1.4.0 (SVN: v12526)
Update Notification:	Receive EMails Don't Receive EMails

Trouble Report Files:

No files

Trouble Report Comments:

#1	cleanrock 02:17 Dec 10, 2016

Create a Fl_Text_Display with wrap_mode(WRAP_AT_BOUNDS, 0) and you should see that the last line is not fully visible after adding one line that wraps. I reverted back to 1.3.3 on archlinux to avoid this issue

#2	greg.ercolano 03:40 Oct 08, 2017

Possibly related bug #3412: http://www.fltk.org/str.php?L3412 Curious if you're using a style table with Fl_Text_Display (usually done to access multi-color text, or multi-font face/size) where the font size in the style table != textsize().

#3	cleanrock 04:12 Oct 08, 2017

Yes I use styles, see src/gui/TextDisplay2.h/cpp at https://github.com/cleanrock/flobby . Thanks for replying but I think you realize how dead/bad FLTK looks if developers ignore bug reports until the by chance run into the same problem themselves.

#4	greg.ercolano 14:12 Oct 08, 2017

Some bugs are slow to be solved, and bugs against Fl_Text_* are just really hard to solve. The lead dev on this particular widget hasn't been around for a while, and this is one of the more complicated widgets even he didn't like to mess with (Matt). Between styles and utf8, and trying to be 'fast', it's tricky. In cases like this, we depend mainly on user supplied patches. And for the rare cases we do get patches, they have to be carefully vetted.

#5	greg.ercolano 10:07 Oct 25, 2017

I believe STR #3412 is related. Try taking 1.4.x svn current, as a fix was made in r12526 that I think addresses this STR too.

#6	cleanrock 04:25 Oct 28, 2017

It seem to be fixed. I tested https://github.com/fltk/test-only (branch-1.4) after fixing Fl_Text_Display ctor which init order in body is totally messed up (just build/run/test with -fsanitize=address and you will see what i mean). I will make a fork/branch available with my changes which include one more fix which will make it possible to have fltk 1.4 as a subdir/submodule so i can link to my fltk 1.4 fork statically.

#7	cleanrock 04:38 Oct 28, 2017

I forgot I also added this to beginning of Fl_Text_Display::calc_line_starts() to avoid crash: if (mBuffer == nullptr) return;

#8	greg.ercolano 18:58 Oct 28, 2017

Re: comment #6, just looking casually, there's a /lot/ of methods that will crash if there's no buffer() defined. Just casually looking, insert(), move_right(), move_down().. yes, calc_line_starts().. pretty much everything. Not sure if we should put tests at the head of all methods that use it, or just document that most of the widget's methods won't work properly unless a buffer() is defined. I did notice the order of initialization in the ctor is a bit unsettling.. both scrollbar pointers remain wild for many lines in the ctor that call other methods that could easily try to manipulate the scrollbars in the future.

#9	cleanrock 19:26 Oct 28, 2017

The return in Fl_Text_Display::calc_line_starts() is needed because it is called from ctor when mBuffer is always null. This is probably a side effect of your change d5ebdb11 "Solves STR#3395.".

#10	greg.ercolano 19:46 Oct 30, 2017

> The return in Fl_Text_Display::calc_line_starts() is needed because > it is called from ctor when mBuffer is always null. > probably a side effect of your change d5ebdb11 "Solves STR#3395." Make sure you're on svn r12530 or higher. What you describe sounds like a problem in r12529 that was fixed by r12530. Sorry, I don't know what git commit r12530 translates to; we're not using git for actual work yet, but it sounds like it's the next consecutive after d5ebdb11.

#11	greg.ercolano 20:19 Oct 30, 2017

Yes, the Fl_Text_Display ctor needs some work. It makes method calls (like textsize()) while many important internal pointers haven't yet been initialized, so that's a bomb waiting to go off during routine code maintenance, as IIRC member variables that are pointers can be /wild/ (and not NULL). We should at /least/ initialize all variables in the ctor before making any method calls, esp. pointers, to ensure nothing is wild.. otherwise tests for buffer() being NULL won't matter if it's a wild value. Will research and follow up.

#12	cleanrock 20:32 Oct 30, 2017

Sounds like you haven't seen my pull request: https://github.com/fltk/test-only/pull/4

#13	greg.ercolano 21:35 Oct 30, 2017

Right: I've not seen the pull request because the devs are not officially using git yet. See Albrecht's comment on the git test-only page: "PLEASE DON'T USE FOR REAL WORK", which includes managing actual bugs. I know it sounds ancient, but until we officially make the switch, please use STRs and submit patches, and only reference svn rev#s in bug reports. Don't use github's issue system or commit hashes.. at least not yet. Regarding the ctor reorg, I've made a separate STR #3422 for that: http://www.fltk.org/str.php?L3422 ..and included a patch suggestion there, to keep that issue separate. That said, I'll check your git diff and compare it to what I've done, and we can follow up on STR #3422, so as not to traffic the current STR with that separate issue.

#14	greg.ercolano 09:10 Nov 02, 2017

Fixed in Subversion repository. Closing this issue as fixed. The separate issue raised in comments #6-#13 have been moved to STR #3422 (ctor reorg), which includes patches I'll probably commit soon. There's I suppose a 3rd issue regarding methods testing mBuffer/buffer() (and possibly other pointers like scrollbars) for NULL, which should be safe to do now that the ctor ensures the members are at least initialized early in the ctor, so methods used later in the ctor can't trip over them. If that becomes a new STR, I'll reference it in #3422.