sciter-sdk / go-sciter

Golang bindings of Sciter: the Embeddable HTML/CSS/script engine for modern UI development
https://sciter.com
2.57k stars 268 forks source link

SetText adds strange spacing before text #266

Closed mrg0lden closed 3 years ago

mrg0lden commented 3 years ago

Hi, I'm trying to get familiar with Sciter, so I tried to call some common DOM functions from Go using Go-sciter.

Using SetText("") inside a "for" loop to change text periodically adds strange spaces before the text everytime the text gets updated.

Notes: Using the inspector, I couldn't spot any space characters being added. Also, the inspector doesn't update with new text when SetText is used. The problem doesn't occur when SetHtml is used, and the updates are instantly reflected on the inspector. When display: inline-block is set on the element, the spacing disappears.

UPDATE: using the inspector's this.text doesn't show the same behavior, it updates the text without the strange space

The code below produces the described incidence.

Click to expand! ### main.go ```go https://user-images.githubusercontent.com/8347078/105359287-545ff280-5c08-11eb-8c53-91bddbfc3781.mp4 package main import ( "time" "github.com/lxn/win" "github.com/sciter-sdk/go-sciter" "github.com/sciter-sdk/go-sciter/window" ) func main() { rect := createRectInTheMiddle(1280, 720) w, err := window.New(sciter.SW_RESIZEABLE|sciter.SW_CONTROLS| sciter.SW_MAIN|sciter.SW_ENABLE_DEBUG, rect) if err != nil { panic(err) } err = w.Sciter.LoadFile("./index.html") if err != nil { panic(err) } w.Show() go func() { root, err := w.GetRootElement() if err != nil { panic(err) } app, err := root.SelectById("app") if err != nil { panic(err) } el, err := sciter.CreateElement("H1", "Hi") if err != nil { panic(err) } app.Append(el) //el.SetStyle("display", "inline-block") Is := "" for i := 0; i < 100; i++ { text := "Hi" Is += "i" el.SetText(text+Is) time.Sleep(time.Second) } }() w.Run() } func createRectInTheMiddle(width, height int) *sciter.Rect { screenWidth := int(win.GetSystemMetrics(win.SM_CXSCREEN)) screenHeight := int(win.GetSystemMetrics(win.SM_CYSCREEN)) top := (screenHeight - height) / 2 left := (screenWidth - width) / 2 return sciter.NewRect(top, left, width, height) } ``` ### index.html ```html Document
```

A video demonstrating the problem.

https://user-images.githubusercontent.com/8347078/105359325-60e44b00-5c08-11eb-81cc-3b3d064a1b92.mp4

pravic commented 3 years ago

Interesting

mrg0lden commented 3 years ago

I want to add that it's not related to the times text gets update, but to the length of the text. I suspect it might be something related to the conversion of Go's UTF-8 strings to UTF-16.

UPDATE: In a fork, I tried to remove the conversion from utf-8 to utf-16, and pass the string as a slice of bytes instead, but it didn't fix the problem. Instead, it introduced another problem related to text encoding (aa -> a japanese or chinese letter.)

https://user-images.githubusercontent.com/8347078/105485639-479dd600-5cbe-11eb-88ed-bf49685f2f25.mp4

pravic commented 3 years ago

cc @c-smile

c-smile commented 3 years ago

@pravic

I do not see problems with calling SciterSetElementText from C++ using this code snippet:

  if (message == WM_KEYDOWN && wParam == VK_F7)
  {
    window* self = ptr(hWnd);
    sciter::dom::element root = self->get_root();
    sciter::dom::element em = root.find_first("em");

    SciterSetElementText(em, L"TEST", 4);

    return 0;
  }

And looking into your Utf16FromString:

func Utf16FromString(s string) ([]uint16, error) {
    for i := 0; i < len(s); i++ {
        if s[i] == 0 {
            return nil, syscall.EINVAL
        }
    }
    return utf16.Encode([]rune(s + "\x00")), nil
}

What is that + "\x00" and how it affects len(result) ? Also, purpose of the loop in that function is murky at best.

mrg0lden commented 3 years ago

Hey, I did some investigations and experimentation, and I'm happy to say that it's solved now.

The notes given by @c-smile were useful to start tinkering.

The function Utf16FromString is originally written as part of the golang.org/x/sys/windows package, see. I tried two things:

  1. Include the "\x00" in the length, so the length is len(result) + 1(this worked ✔)
  2. Remove the "\x00" and keep the length as is (this didn't work, I think this \x00 is the null termination of the string)

You can try it by using go get github.com/mrg0lden/go-sciter@399f99827a0831d72a7ebc2480066930c8f53a50 which gets the commit with the selected hash. It's the commit with the "trying2" comment here.

I also had to add a go.mod file to avoid some annoyances, and changed the import path to "github.com/mrg0lden/go-sciter" to use my commit.

c-smile commented 3 years ago

Case number 2 shall work as far as I can tell.

In any case, when Sciter passes character range it passes start and length that does not include zero terminator. The same way on input - when character range is passed to it - it expects length to contain exact number of characters - not including that zero.

I do not see here that utf16.Encode requires zero termination
https://sourcegraph.com/github.com/golang/go/-/blob/src/unicode/utf16/utf16.go#L56

mrg0lden commented 3 years ago

Well, it seems case number 2 didn't work for another reason, some functions depend on UTF16FromString with its current behavior.

c-smile commented 3 years ago

Utf16FromString may append zero terminator if that is needed.

But in cases where you pass pair of const char16_t* start, UINT length to Sciter API you shall pass that length without zero terminator.

mrg0lden commented 3 years ago

I'll try another approach then and report the results

mrg0lden commented 3 years ago

I tried using the package unicode/utf16 directly to encode text, it worked flawlessly. UPDATE: with length=0; it casus a go panic panic: runtime error: index out of range [0] with length 0 when text is set to ""

pravic commented 3 years ago

Thanks!