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

TextBoxWidget: Unbreak Wikipedia #8275

Merged
merged 1 commit into from
Sep 27, 2021
Merged

TextBoxWidget: Unbreak Wikipedia #8275

merged 1 commit into from
Sep 27, 2021

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Sep 27, 2021

Turns out we can't release line_num_to_image early, so,
delay it until the final, full widget teardown.

(Regression since #8250, c.f.,
#8250 (comment))


This change is Reviewable

Turns out we can't release line_num_to_image early, so,
delay it until the final, full widget teardown.

(Regression since koreader#8250, c.f.,
koreader#8250 (comment))
@NiLuJe NiLuJe merged commit 0ea8bee into koreader:master Sep 27, 2021
@Frenzie Frenzie added this to the 2021.10 milestone Sep 28, 2021
@poire-z
Copy link
Contributor

poire-z commented Oct 4, 2021

In this week's Turns out we can't.... episode:

./luajit: frontend/ui/widget/textboxwidget.lua:771: attempt to get length of field 'vertical_string_list' (a nil value)
stack traceback:
        frontend/ui/widget/textboxwidget.lua:771: in function '_renderText'
        frontend/ui/widget/textboxwidget.lua:867: in function '_updateLayout'
        frontend/ui/widget/textboxwidget.lua:1156: in function 'update'
        frontend/ui/widget/textboxwidget.lua:998: in function 'action'

I can't find out 100% reproducible steps. But twice I got it via Wikipedia history > hit past lookup "foo" > browse 2 results with images, Wikipedia full on the 2nd ("Dave Grohl"), close the full result, and crash.

So, @ N̷i̼̪̪̖͖ḽ̣̠̺̜'̪̯̘̙̱͞u̧͖̭̜͚̪j͙̝̩̝̪̖e͉͝ T̻͓̺͎̱͙̫h̹͇e̛ ̗̭N̤͉͎͉̬il̠̱͡'̼i̢̙̟̝̹͉f҉̙̝̝͉icà̳̰̠̬͚t̹̼̻͟ò͓͈͓̥r҉̫̘͍ ͚̹̞ͅ ! I implore you in fear and trembling: š͚̪̐ṯ̻̥̹̓ͮ̿ͤ̄o̖ͧͤͭ̑̒̽̓ͅp̦̱͎̳͍̣̞ͩ̄͐̾ ͍̙̺̗̆̿̅ͧͯ̑̓ṇ̗i̳̖̰l̗ͅ'̳̭͔͕̤̤̉͊͌̂ĩ̱͔̠̫̒̀f̤̗̮̖̤͈̒ͅý̪̖̎͆̉̚í̼n̥̮̪̑ͧ̎͋̽ͅg̙̜͕͍͚͒̅̔̍ ̬͇ͨ̍ͧ̒ͮ̇e̪̹̼̩̦ͩ̍v̖̘̑̔́͊ͯͮ̚e̪͖͙̝͐͐ͥ͆̊r̦̲̖̘̰̦̊y͈͔̅̃ͤͮͪ̅̔t̞̝̃̃̒ͤ̉ͤh̫͈̝͎̬̙͐̎̇̌̑̿i̓n͉̼̟̟̫̰̉ͫ̊ͬ͋ͪͩgͧ̈́ !

@NiLuJe
Copy link
Member Author

NiLuJe commented Oct 4, 2021

That... looks like an actual bug (i.e., a widget reused after a full :free(true) and not re-init'ed?).

@NiLuJe
Copy link
Member Author

NiLuJe commented Oct 4, 2021

Oh, wait, no. _splitToLines uses a blank table to reset, not a nil ;).

NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Oct 4, 2021
@poire-z
Copy link
Contributor

poire-z commented Oct 4, 2021

An empty table instead of a nil, you're getting soft :)
(Still can't manage to reproduce it, so can't check if this would have solved it.)

NiLuJe added a commit that referenced this pull request Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants