Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#5681 closed patch (out of date)

infinite input area auto resizing, pidgin hangs.

Reported by: irtiger Owned by: sadrul
Milestone: Component: pidgin (gtk)
Version: 2.4.1 Keywords:
Cc:

Description

When input complex characters such as korean or chinese using scim, pidgin falls into infinite loop when input area auto-resizing occurs.

Here is a test scenario:

  1. Setting SCIM language to Korean or Chinese.
  2. Fill those complex characters(not ascii) in pidgin's conversation input area to be auto-resized.
  3. Infinit auto-resizing, input area widget shakes.

To see what happens, I attached to the process when auto-resize occurs. It looks like gtkconv.c:resize_imhtml_cb() function is problem. Once resize occurs, resize_imhtml_cb() is called infinitely so it makes pidgin hang.

Attaching manually gdb-traced results. I put display of meaningful values for each continue-stop that may helps understand why.

Attachments (3)

pidgin_gdbtrace.txt (25.6 KB) - added by irtiger 11 years ago.
gdb tracelog
pidgin-fix-infinite-auto-resize.patch (2.2 KB) - added by tejun 11 years ago.
reimplement resize_imhtml_cb() using gtk_text_layout_get_size() to fix infinite auto-resize
pidgin-update-resize_imhtml_cb.patch (3.0 KB) - added by tejun 11 years ago.
pidgin-update-resize_imhtml_cb.patch

Download all attachments as: .zip

Change History (16)

Changed 11 years ago by irtiger

gdb tracelog

comment:1 Changed 11 years ago by datallah

  • Owner set to seanegan

comment:2 follow-up: Changed 11 years ago by sadrul

Could you please print the value of oneline.height in gtkconv.c:4397?

comment:3 in reply to: ↑ 2 Changed 11 years ago by irtiger

Replying to sadrul:

Could you please print the value of oneline.height in gtkconv.c:4397?

oneline.height is always 17.

Changed 11 years ago by tejun

reimplement resize_imhtml_cb() using gtk_text_layout_get_size() to fix infinite auto-resize

comment:4 Changed 11 years ago by tejun

Attached is the patch to fix the problem. It's against pidgin-2.4.1. The following is why it happens.

  1. Conversation entry area is about to overflow and a complex character is entered via IM and put into preedit area of TextEntry?.
  1. resize_imhtml_cb() is invoked. It iterates through the text buffer display-line-by-display-line counting display lines. Although the display line iteration itself does consider characters in preedit area, the preedit area is not part of the text buffer, so the iterator reaches end where the text buffer ends and the preedit area starts. So, the display line count is off by -1.
  1. resize_imhtml_cb() requests resize and finishes.
  1. gtk goes out and resizes the area. It of course considers the pre-edit area. As the text view is too small to accommodate text buffer + preedit area, vscroll is created, which reduces its width and thus overflowing characters at the end of the text buffer into the next line.
  1. resize_imhtml_cb() is invoked again. As the width has been shrank, the end of the text buffer overflowed into the next line along with the preedit area. So, this time, the line count is correct.
  1. resize_imhtml_cb() requests resize and finishes.
  1. gtk resizes again. As the height is now enough to contain everything, it removes the vscroll and the width is restored.
  1. goto 2, ad nauseam.

The problem is that there's no way to access or determine the size of the preedit area via TextView? interface. The patch uses unsupported gtk_text_layout_get_size(), which returns the final size of the rendered area to work around the problem. It's cleaner and more efficient. IMHO, proper solution would be adding similar interface to TextView?.

Thanks && kudos for the great work, can't live without pidgin :-)

comment:5 Changed 11 years ago by datallah

  • Type changed from defect to patch

comment:6 Changed 11 years ago by tejun

Ping.

comment:7 Changed 11 years ago by sadrul

  • Owner changed from seanegan to sadrul
  • Status changed from new to assigned

The explanation looks good, as does the patch. I was a little hesitant about the use of GTK_TEXT_USE_INTERNAL_UNSUPPORTED_API, but it looks like its use is not that rare, and the resulting code from its use is much cleaner. I am going to commit this one.

tejun: What name+email should I use for the commit/changelog?

comment:8 Changed 11 years ago by tejun

Please use Tejun Heo <tj@…>. Thanks.

comment:9 Changed 11 years ago by rekkanoryo

Did this patch ever get committed?

comment:10 Changed 11 years ago by rekkanoryo

tejun, can you update this patch to be against either 2.5.2 or current monotone?

Changed 11 years ago by tejun

pidgin-update-resize_imhtml_cb.patch

comment:11 Changed 11 years ago by tejun

Okay, here's updated patch against 2.5.2. Several things to note...

  1. The infinite-resize bug doesn't exist anymore even w/o the patch. The new gtk_text_view_get_line_yrange() based implementation seems to work fine. I still think asking gtk about the widget size is better/simpler approach except for the UNSUPPORTED_API stunt. So, one pro and one con, the decision is up to you.
  1. In the above posted patch, I restored the original "diff == 0 && (diff < 0 && -diff < oneline.height / 2)" condition because the thing gotta grow whether the amount of growth is bigger than half a line or not and with the gtk telling us the true size of the widget, it should just work fine. I didn't go through the log, so if the ABS() check was added for some different reason, you might wanna drop that part.

Thanks.

comment:12 Changed 11 years ago by rekkanoryo

  • Resolution set to out of date
  • Status changed from new to closed

If this is fixed without this patch, then there's no reason to apply the patch. Sorry to have had you do unnecessary work.

comment:13 Changed 10 years ago by QuLogic

Ticket #5547 has been marked as a duplicate of this ticket.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!