invisible-college / tawk.space

Social video chats
https://tawk.space
Apache License 2.0
14 stars 1 forks source link

Tawk.space freezes sometimes when scratch text area needs to expand #28

Closed tkriplean closed 7 years ago

tkriplean commented 7 years ago

Behavior:

Replication:

toomim commented 7 years ago

I can't reproduce this. I made a four-person tawk-space (by opening four tabs of myself), pasted in 5833 lines of text, and it worked just fine. Can you reproduce this with another browser? If so, can you give instructions that we can try on our computers?

tkriplean commented 7 years ago

I haven't been able to reliably reproduce it. I also tried opening tawk space in multiple browsers to reproduce, but that didn't do it.

Michael Toomim wrote:

I can't reproduce this. I made a four-person tawk-space (by opening four tabs of myself), pasted in 5833 lines of text, and it worked just fine. Can you reproduce this with another browser? If so, can you give instructions that we can try on our computers?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/invisible-college/tawk.space/issues/28#issuecomment-280976100, or mute the thread https://github.com/notifications/unsubscribe-auth/AArJ3zMQ33DpgABmiCPBa1VLdBsabj5fks5reQAjgaJpZM4MEuCI.

-- Travis Kriplean, Ph.D Creator ofConsider.it http://consider.it/

karth295 commented 7 years ago

I couldn't reproduce it either -- let's try at the next invisible college meet.

toomim commented 7 years ago

Looks like it might happen when pasting some types of rich text. Maybe unicode or something.

tkriplean commented 7 years ago

This happened again. Here are some more observations. Still not reproducible.

toomim commented 7 years ago

Did you guys figure out what was causing the bug? I'd like to know.

karth295 commented 7 years ago

It's hard to reproduce, so we never figured out why the bug was happening. However, 99% sure one of these loops ends up being an infinite loop. Chrome eventually complains that the tab is unresponsive.

while(target.rows > 1 && target.scrollHeight < target.offsetHeight)
  target.rows--
while(target.scrollHeight > target.offsetHeight)
  target.rows++
toomim commented 7 years ago

I'd like to put print statements in there to see what the problem is:

count = 0
while(target.rows > 1 && target.scrollHeight < target.offsetHeight)
  target.rows--
  if count++ > 1000 then throw "we triggered an autoresize bug: #{target} #{target.rows} #{target.scrollHeight} #{target.offsetHeight}"

while(target.scrollHeight > target.offsetHeight)
  target.rows++
  if count++ > 1000 then throw "we triggered an autoresize bug: #{target} #{target.rows} #{target.scrollHeight} #{target.offsetHeight}"

This code is much smaller than the GROWING_TEXTAREA version, and I'd like to include it in our statebus libraries.

karth295 commented 7 years ago

Okay, I switched back and added the throw statements in on https://tawk.space.

toomim commented 7 years ago

If there's an infinite loop bug there, it must be in the second loop, because the first loop exits if target.rows <= 1, and it decrements target.rows in each loop.

toomim commented 7 years ago

Cool :) thanks.

If the bug is in the second loop, here's an explanation of what might be happening:

scrollHeight is the number of pixels of text inside the textbox, even if it must be scrolled to see it all. offsetHeight is the pixels of the textbox that wraps the text. rows is the number of lines of text the box is displaying.

So this code increases the number of rows until the scrollable text size is smaller than the textbox. Perhaps what's happening is that chrome is not actually redrawing the box (on travis's machine, in some situations) to realize that the offsetHeight is larger. We could catch this by watching for when offsetHeight is not changing in a loop, like this:

old_offsetHeight = target.offsetHeight
while(target.scrollHeight > target.offsetHeight)
  target.rows++
  if old_offsetHeight == target.offsetHeight
    console.log('offsetheight', old_offsetHeight, 'didn't change')
    return
  if count++ > 1000 then throw "we triggered an autoresize bug: #{target} #{target.rows} #{target.scrollHeight} #{target.offsetHeight}"
karth295 commented 7 years ago

Okay, I added that.

tkriplean commented 7 years ago

I figured out the bug:

AUTOSIZEBOX depends on adjusting textarea.rows to adjust height. However, if the user manually adjusts the textarea by dragging the lower right corner, textarea.rows no longer controls the textarea's height. Thus the loop that increases a textarea's rows until it is big enough is infinite.

I'm now using this code:

resizebox = (target) ->
  while(target.rows > 1 && target.scrollHeight < target.offsetHeight)
    target.rows--
  while(target.scrollHeight > target.offsetHeight)
    target.rows++
    if target.rows > 10000
      console.error 'Infinite loop detected in AUTOSIZEBOX. Exiting.'
      return
toomim commented 7 years ago

That's great. Looks like what's happening is:

  1. dragging the resize handle sets an inline style="width: x, height: y" attribute
  2. css width and height overrides the rows/cols property
  3. you can disable the resize handle with style="resize: 'none'"

So there are two ways to fix this:

  1. disable resize
  2. or clear the inline width and height when it resizes

Resize can be disabled vertically, horizontally, or both. I've disabled resize vertically, and if the programmer specifies a width, then horizontally too.

And each time it refreshes, it clears any latent style.height that might override the rows.

Here's the full code:

dom.AUTOSIZEBOX = ->
  @props.style.resize = if @props.style.width or @props.cols then 'none' else 'horizontal'
  TEXTAREA
    ref: 'textbox'
    rows: 1
    cols: @props.cols
    placeholder: @props.placeholder
    onKeyDown: (e) =>@props.onKeyDown?(e)
    onChange: (e) => @props.onChange?(e)
    className: @props.className
    style: @props.style
    value: @props.value

resizebox = (target) ->
  target.style.height = null
  while(target.rows > 1 && target.scrollHeight < target.offsetHeight)
    target.rows--
  while(target.scrollHeight > target.offsetHeight)
    target.rows++

dom.AUTOSIZEBOX.up      = -> resizebox(@refs.textbox.getDOMNode())
dom.AUTOSIZEBOX.refresh = -> resizebox(@refs.textbox.getDOMNode())

Committed: https://github.com/invisible-college/tawk.space/commit/90fe06f7911e94c945e705d7b4ade8562a02d958