[haiku-commits] Re: haiku: hrev47318 - src/apps/terminal

  • From: Ingo Weinhold <ingo_weinhold@xxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 05 Jun 2014 23:55:03 +0200

On 05.06.2014 22:22, jscipione@xxxxxxxxx wrote:
############################################################################

Commit:      b84955c73892d942d2b53e7a992b63e774e52789
URL:         http://cgit.haiku-os.org/haiku/commit/?id=b84955c
Author:      John Scipione <jscipione@xxxxxxxxx>
Date:        Thu Jun  5 20:09:52 2014 UTC

TermViewStates: Check if fView is NULL before using it.

Could you please try to at least somewhat understand code before making changes? fView is set in the base class constructor and never changed. Please revert.

############################################################################

Revision:    hrev47318
Commit:      4c8d2387169eb503e9a4a347645dd37c238d1c8a
URL:         http://cgit.haiku-os.org/haiku/commit/?id=4c8d238
Author:      John Scipione <jscipione@xxxxxxxxx>
Date:        Thu Jun  5 20:18:49 2014 UTC

Ticket:      https://dev.haiku-os.org/ticket/10902

Fix #10902 Terminal crashes setting window size.

Terminal crashes because fView is not connected to App Server when
this is called so calling fView->GetMouse() is not allowed.

The interesting questions therefore are:

1) Why is the view not attached?
2) Why is it in the hyper link state?

1) is because the BTabView removes the non-selected tabs instead of just hiding them.

The reason for 2) is that the tab was opened with Cmd+T (the bug is not reproducible when the tab is opened via menu item). Pressing Cmd causes hyper link state to be entered and switching to the new tab will leave the view in that state due to 1).

Possible solutions:

* TermView::_VisibleTextBufferChanged(): Call state hook only when attached to window. All other occurrences are safe as they are in BView hooks.

* Leave the hyper link state when the view is detached from the window. A new dummy state could be active as long as the view is not attached, though using DefaultState would be harmless as well.

We could
perhaps check to make sure that fView is connected to App Server by
checking that fView->Window() != NULL, but, a better way to handle this
is to grab the already set fLastClickPoint variable from fView
which eliminates the need to grab the current mouse position again.

fLastClickPoint is, as the name suggests, the point where the user last clicked (actually the point of the last MouseDown()), not the current mouse position.

Worst case scenario is that fLastClickPoint hasn't been set in which
case Terminal won't find anything to highlight or unhighlight.

No, the worst case scenario is that the hyper link feature is now partially broken. E.g. pressing Cmd while the mouse is over a path will no longer highlight the path.

Also check to make sure that fView != NULL here for good measure.

Yeah, that will help.

CU, Ingo


Other related posts: