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

nil pointer dereference in dialog.Resize() for color picker #5236

Closed
2 tasks done
ole108 opened this issue Nov 4, 2024 · 3 comments
Closed
2 tasks done

nil pointer dereference in dialog.Resize() for color picker #5236

ole108 opened this issue Nov 4, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@ole108
Copy link

ole108 commented Nov 4, 2024

Checklist

  • I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
  • This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

A nil pointer dereference occurs when resizing a color picker dialog.
This only seems to only happen without setting an initial color with colorPicker.SetColor(c).
The real nil pointer occurs in popup.go.

It can be worked around with calling colorPicker.Refresh() before resizing.

How to reproduce

  1. Create a color picker dialog (cp).
  2. Do not set the initial color with cp.SetColor().
  3. Call cp.Resize()
  4. Call cp.Show()

Do the steps 1. to 4. again but with the second step modified to:
2. Do set set the initial color with cp.SetColor().

The third step (Resize) will crash the app.

Screenshots

No response

Example code

package main

import (
	"fmt"
	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/dialog"
	"image/color"
	"os"
)

func main() {
	fapp := app.NewWithID("app-with-unique-id")
	win := fapp.NewWindow("pick color")
	win.Resize(fyne.NewSize(500, 500))

	cp := dialog.NewColorPicker("", "", func(c color.Color) {
		fmt.Println("Color callback called!")
		win.Close()
		fapp.Quit()
	}, win)
	if len(os.Args) >= 2 && os.Args[1] == "1" {
		cp.Advanced = true
		cp.SetColor(color.White)
	}
	if len(os.Args) >= 3 && os.Args[2] == "2" {
		cp.Refresh() // update the picker internal UI
	}
	cp.Resize(fyne.NewSize(500, 500)) // this might crash!!!

	cp.Show()
	win.Show()
	fapp.Run()
}

Fyne version

2.5.2

Go compiler version

1.23.1

Operating system and version

Linux 6.1.0-26-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.112-1 (2024-09-30) x86_64 GNU/Linux

Additional Information

The fix is rather simple.
Just call p.updateUI() in an own Resize method for the color picker before calling the base dialogs Resize method.
That will fill p.win and so prevent win from being nil.

I would be happy to provide a pull request.

@ole108 ole108 added the unverified A bug that has been reported but not verified label Nov 4, 2024
@saravanan-golang
Copy link

saravanan-golang commented Dec 4, 2024

Screenshot 2024-12-04 125256

cp.Resize(fyne.NewSize(500, 500)) should be called after cp.Show()

Since the Resize method should be called only after the Show method
If done so the code doesn't crash @ole108

FYI: @andydotxyz

@andydotxyz
Copy link
Member

Since the Resize method should be called only after the Show method

We don't want to make any requirements about method order call. It should work if you resize before show and it certainly should not crash if you do!

@andydotxyz
Copy link
Member

Fixed on develop branch

@andydotxyz andydotxyz added bug Something isn't working and removed unverified A bug that has been reported but not verified labels Dec 4, 2024
@andydotxyz andydotxyz added this to the E fixes (v2.5.x) milestone Dec 4, 2024
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

3 participants