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

Entry.SetText may panic if called on a multiline entry with selected text #2482

Closed
nullst opened this issue Sep 19, 2021 · 0 comments
Closed
Labels
bug Something isn't working

Comments

@nullst
Copy link
Contributor

nullst commented Sep 19, 2021

Describe the bug:

Suppose a multiline entry has a long text in it, with some part selected. If SetText() is called on the entry with a new text shorter than the original text, the selected region may be out of bounds of the new text. I don't know what the proper behavior should be, but as of now this results in a nil pointer dereference originating from the function entryContentRendered.buildSelection in widget/entry.go. An examination in debugger shows that in that function the object r.content.entry has:

  1. Updated (short) text.
  2. Updated cursor position (at the very end of the new text since previous cursor position was even further).
  3. Old values of selectRow and selectColumn (i.e., specifying a position that does not exist in the new text).

This leads to a wrong computation of rowCount, and eventually lineSizeToColumn is called with an out of bounds arguments.

Notes:

  1. At some moment before the segmentation fault the cursor position was modified to be consistent with the new text, but the selected position was not. Maybe this happens in entryRenderer.Refresh(), but I'm not sure.
  2. I've originally discovered this when .SetText() was called while I, as a user, was in the process of changing the selection state by dragging the mouse. Probably any fix would cover this situation as well.
  3. This happens regardless of whether the multiline entry has wrapping enabled.

To Reproduce:

Steps to reproduce the behaviour:

  1. Compile the example code below and run it.
  2. Scroll down a bit and select some text.
  3. Wait 5 seconds until the timer calls entry.SetText().
  4. See segmentation fault:
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x447bff5]

goroutine 22 [running]:
fyne.io/fyne/v2/widget.(*RichText).lineSizeToColumn(0xc00051a000, 0x0, 0x1, 0xc000132b5c)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/richtext.go:244 +0x55
fyne.io/fyne/v2/widget.(*entryContentRenderer).buildSelection.func1(0x0, 0x1, 0x0)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/entry.go:1533 +0x45
fyne.io/fyne/v2/widget.(*entryContentRenderer).buildSelection(0xc000136050)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/entry.go:1583 +0x346
fyne.io/fyne/v2/widget.(*entryContentRenderer).moveCursor(0xc000136050)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/entry.go:1624 +0x2f
fyne.io/fyne/v2/widget.(*entryContentRenderer).Refresh(0xc000136050)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/entry.go:1497 +0x186
fyne.io/fyne/v2/widget.(*entryRenderer).Refresh(0xc000068180)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/entry.go:1371 +0x3f0
fyne.io/fyne/v2/widget.(*BaseWidget).Refresh(0xc000490480)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/widget.go:138 +0x5c
fyne.io/fyne/v2/widget.(*BaseWidget).setFieldsAndRefresh(0xc000490480, 0xc000494758)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/widget.go:152 +0x82
fyne.io/fyne/v2/widget.(*Entry).updateText(0xc000490480, 0x45e174a, 0x48)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/entry.go:1156 +0x89
fyne.io/fyne/v2/widget.(*Entry).SetText(0xc000490480, 0x45e174a, 0x48)
	/Users/nullst/fun/programming/fyne-fix/fyne/widget/entry.go:470 +0x3f
main.main.func1.1()
	/Users/nullst/fun/programming/fyne-settext-race/main.go:35 +0x45
created by time.goFunc
	/usr/local/go/src/time/sleep.go:167 +0x45

Example code:

package main

import (
	"time"

	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/widget"
)

var textFragment string = "ihsidf hasd fihasdf jas dfuias fduias fuibasd fibas dfoih kjhkhk uhuhuih"

var useWrapping bool = false

func main() {
	app := app.New()
	w := app.NewWindow("Entry with updating text")

	var longText string
	for i := 0; i < 60; i++ {
		longText += textFragment
		if !useWrapping {
			longText += "\n"
		}
	}

	entry := widget.NewMultiLineEntry()
	if useWrapping {
		entry.Wrapping = fyne.TextWrapWord
	}
	entry.SetText(longText)

	go func() {
		time.AfterFunc(5*time.Second, func() {
			entry.SetText(textFragment)
		})
	}()

	w.SetContent(entry)
	w.Resize(fyne.NewSize(float32(500), float32(300)))
	w.ShowAndRun()
}

Device (please complete the following information):

  • OS: Mac OS
  • Version: 10.12 Sierra
  • Go version: go1.15.3 darwin/amd64
  • Fyne version: 6e90820
@nullst nullst added the unverified A bug that has been reported but not verified label Sep 19, 2021
@andydotxyz andydotxyz added bug Something isn't working and removed unverified A bug that has been reported but not verified labels Sep 21, 2021
@andydotxyz andydotxyz added this to the Fixes (v2.1.x) milestone Sep 21, 2021
nullst added a commit to nullst/fyne that referenced this issue Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants