Scott Kostyshak wrote: > On Fri, Jan 12, 2018 at 05:20:18PM +0000, Pavel Sanda wrote: > > Pavel Sanda wrote: > > > after pushing the outliner button we call in TocWidget::outline generic > > > GuiApplication::dispatch which likely grabs currently active window; if > > > that's > > > the wrong window, it grabs wrong cursor position, moves wrong section and > > > suddenly content of outliner and buffer structure are out of sync and > > > crash is > > > just matter of time.... > > > > > > Now this is not some glitch but rather conceptual problem how we handle > > > lfuns from the widget(s). > > > Opinions how to move forward? > > > > E.g. I could implement requested_guiview inside FuncRequest so we could > > specify window > > as the lfun request travels through GuiApplication. Any objections to this > > solution? > > Bump. > > Scott
I have patch along the lines above, i.e. when we request lyx::dispatch we also provide guiview inside FuncRequest, so we can at least detect in guiapplication::dispatch that current_view is different and stop there. So this crash is gone. Various attempts with raise/activewindow/setcurrentview for getting the right window up in the first place did not seem to work. But it is difficult to be sure, the scenario of multiple windows+undocked outliner is minefield with crashes stemming from different reasons. There is immediately different crash - apparently when we load already opened buffer in new window, the load resets tocwidget from the oldwindow and if you click to some toc item without focusing old window first you often get crash, see below. Thread 1 "lyx" received signal SIGSEGV, Segmentation fault. 0x00005555557742fd in lyx::CursorSlice::text (this=<optimized out>, this=<optimized out>) at CursorSlice.h:119 119 Text * text() const { return inset_->getText(idx_); } (gdb) bt #0 0x00005555557742fd in lyx::CursorSlice::text (this=<optimized out>, this=<optimized out>) at CursorSlice.h:119 #1 lyx::DocIterator::innerTextSlice (this=0x7fffffffcbd0) at DocIterator.cpp:233 #2 0x0000555555774419 in lyx::DocIterator::paragraphGotoArgument[abi:cxx11]() const (this=<optimized out>) at DocIterator.cpp:247 #3 0x00005555558ae1eb in lyx::TocItem::action (this=this@entry=0x7fffffffcbd0) at TocBackend.cpp:75 #4 0x0000555555bbcc5f in lyx::frontend::TocModels::goTo (this=<optimized out>, type=..., index=...) at TocModel.cpp:316 #5 0x0000555555d1ebc6 in lyx::frontend::TocWidget::goTo (this=this@entry=0x555556b42330, index=...) at TocWidget.cpp:252 #6 0x0000555555d1ec0b in lyx::frontend::TocWidget::on_tocTV_pressed (this=0x555556b42330, index=...) at TocWidget.cpp:240 ... I saw that when loading the buffer in new window the new guiview points to buffer, which has pointer to old guiview. That trigers toc updates in old window. Is this expected??? I seem to be somewhat lost why buffer has pointer to guiview at all when multiple windows are possible. Pavel
diff --git a/src/FuncRequest.cpp b/src/FuncRequest.cpp index cd47c55595..ef357ee447 100644 --- a/src/FuncRequest.cpp +++ b/src/FuncRequest.cpp @@ -31,38 +31,39 @@ FuncRequest const FuncRequest::noaction(LFUN_NOACTION); FuncRequest::FuncRequest(Origin o) : action_(LFUN_NOACTION), origin_(o), x_(0), y_(0), - button_(mouse_button::none), modifier_(NoModifier) + button_(mouse_button::none), modifier_(NoModifier), view_origin_(0) {} FuncRequest::FuncRequest(FuncCode act, Origin o) : action_(act), origin_(o), x_(0), y_(0), - button_(mouse_button::none), modifier_(NoModifier) + button_(mouse_button::none), modifier_(NoModifier), view_origin_(0) {} FuncRequest::FuncRequest(FuncCode act, docstring const & arg, Origin o) : action_(act), argument_(arg), origin_(o), x_(0), y_(0), - button_(mouse_button::none), modifier_(NoModifier) + button_(mouse_button::none), modifier_(NoModifier), view_origin_(0) {} FuncRequest::FuncRequest(FuncCode act, string const & arg, Origin o) : action_(act), argument_(from_utf8(arg)), origin_(o), x_(0), y_(0), - button_(mouse_button::none), modifier_(NoModifier) + button_(mouse_button::none), modifier_(NoModifier), view_origin_(0) {} FuncRequest::FuncRequest(FuncCode act, int ax, int ay, mouse_button::state but, KeyModifier modifier, Origin o) : action_(act), origin_(o), x_(ax), y_(ay), button_(but), - modifier_(modifier) + modifier_(modifier), view_origin_(0) {} FuncRequest::FuncRequest(FuncRequest const & cmd, docstring const & arg, Origin o) : action_(cmd.action()), argument_(arg), origin_(o), - x_(cmd.x_), y_(cmd.y_), button_(cmd.button_), modifier_(NoModifier) + x_(cmd.x_), y_(cmd.y_), button_(cmd.button_), modifier_(NoModifier), + view_origin_(0) {} diff --git a/src/FuncRequest.h b/src/FuncRequest.h index 20cd96ab66..80587f9f01 100644 --- a/src/FuncRequest.h +++ b/src/FuncRequest.h @@ -24,6 +24,10 @@ namespace lyx { class LyXErr; +namespace frontend { + class GuiView; +} + /** * This class encapsulates a LyX action and its argument * in order to pass it around easily. @@ -70,6 +74,10 @@ public: /// void setOrigin(Origin o) { origin_ = o; } /// + frontend::GuiView* view_origin() const { return view_origin_; } + /// + void setViewOrigin(frontend::GuiView* o) { view_origin_ = o; } + /// int x() const { return x_; } /// int y() const { return y_; } @@ -97,6 +105,9 @@ private: docstring argument_; /// who initiated the action Origin origin_; + /// to which view should be this command sent (see bug #11004) + /// NULL=current view + frontend::GuiView* view_origin_; /// the x coordinate of a mouse press int x_; /// the y coordinate of a mouse press diff --git a/src/frontends/qt4/GuiApplication.cpp b/src/frontends/qt4/GuiApplication.cpp index 97a60e9b44..4e619b501f 100644 --- a/src/frontends/qt4/GuiApplication.cpp +++ b/src/frontends/qt4/GuiApplication.cpp @@ -1391,7 +1391,16 @@ static docstring makeDispatchMessage(docstring const & msg, DispatchResult const & GuiApplication::dispatch(FuncRequest const & cmd) { + DispatchResult dr; + Buffer * buffer = 0; + if (cmd.view_origin() && current_view_ != cmd.view_origin()) { + //setCurrentView(cmd.view_origin); //does not work + dr.setError(true); + dr.setMessage(_("Wrong focus!")); + d->dispatch_result_ = dr; + return d->dispatch_result_; + } if (current_view_ && current_view_->currentBufferView()) { current_view_->currentBufferView()->cursor().saveBeforeDispatchPosXY(); buffer = ¤t_view_->currentBufferView()->buffer(); @@ -1399,7 +1408,6 @@ DispatchResult const & GuiApplication::dispatch(FuncRequest const & cmd) buffer->undo().beginUndoGroup(); } - DispatchResult dr; // redraw the screen at the end (first of the two drawing steps). // This is done unless explicitly requested otherwise dr.screenUpdate(Update::FitCursor); diff --git a/src/frontends/qt4/TocModel.cpp b/src/frontends/qt4/TocModel.cpp index 0733c1f641..51365311aa 100644 --- a/src/frontends/qt4/TocModel.cpp +++ b/src/frontends/qt4/TocModel.cpp @@ -303,17 +303,17 @@ QModelIndex TocModels::currentIndex(QString const & type, } -void TocModels::goTo(QString const & type, QModelIndex const & index) const +FuncRequest TocModels::goTo(QString const & type, QModelIndex const & index) const { const_iterator it = models_.find(type); if (it == models_.end() || !index.isValid()) { LYXERR(Debug::GUI, "TocModels::goTo(): QModelIndex is invalid!"); - return; + return FuncRequest(LFUN_NOACTION); } - LASSERT(index.model() == it.value()->model(), return); + LASSERT(index.model() == it.value()->model(), return FuncRequest(LFUN_NOACTION)); TocItem const item = it.value()->tocItem(index); LYXERR(Debug::GUI, "TocModels::goTo " << item.asString()); - dispatch(item.action()); + return item.action(); } diff --git a/src/frontends/qt4/TocModel.h b/src/frontends/qt4/TocModel.h index ed94218ad6..8d336399c1 100644 --- a/src/frontends/qt4/TocModel.h +++ b/src/frontends/qt4/TocModel.h @@ -22,6 +22,7 @@ namespace lyx { class Buffer; class BufferView; class DocIterator; +class FuncRequest; namespace frontend { @@ -124,9 +125,7 @@ public: QModelIndex currentIndex(QString const & type, DocIterator const & dit) const; /// - void goTo(QString const & type, QModelIndex const & index) const; - /// - void init(Buffer const & buffer); + FuncRequest goTo(QString const & type, QModelIndex const & index) const; /// void updateItem(QString const & type, DocIterator const & dit); /// diff --git a/src/frontends/qt4/TocWidget.cpp b/src/frontends/qt4/TocWidget.cpp index b96a7c644c..dcd9c4d0e5 100644 --- a/src/frontends/qt4/TocWidget.cpp +++ b/src/frontends/qt4/TocWidget.cpp @@ -178,6 +178,7 @@ bool TocWidget::getStatus(Cursor & cur, FuncRequest const & cmd, void TocWidget::doDispatch(Cursor & cur, FuncRequest const & cmd) { + Inset * inset = itemInset(); FuncRequest tmpcmd(cmd); @@ -235,6 +236,7 @@ void TocWidget::on_tocTV_activated(QModelIndex const & index) void TocWidget::on_tocTV_pressed(QModelIndex const & index) { + Qt::MouseButtons const button = QApplication::mouseButtons(); if (button & Qt::LeftButton) { goTo(index); @@ -249,7 +251,7 @@ void TocWidget::goTo(QModelIndex const & index) LYXERR(Debug::GUI, "goto " << index.row() << ", " << index.column()); - gui_view_.tocModels().goTo(current_type_, index); + sendDispatch(gui_view_.tocModels().goTo(current_type_, index)); } @@ -314,6 +316,7 @@ void TocWidget::setTreeDepth(int depth) void TocWidget::on_typeCO_currentIndexChanged(int index) { + if (index == -1) return; current_type_ = typeCO->itemData(index).toString(); @@ -328,14 +331,29 @@ void TocWidget::outline(FuncCode func_code) QModelIndexList const & list = tocTV->selectionModel()->selectedIndexes(); if (list.isEmpty()) return; + + //if another window is active, this attempt will fail, + //but it will work at least for the second attempt + gui_view_.activateWindow(); + enableControls(false); goTo(list[0]); - dispatch(FuncRequest(func_code)); + sendDispatch(FuncRequest(func_code)); enableControls(true); gui_view_.setFocus(); } +void TocWidget::sendDispatch(FuncRequest fr) +{ + + fr.setViewOrigin(&gui_view_); + DispatchResult dr=dispatch(fr); + if (dr.error()) + gui_view_.message(dr.message()); +} + + void TocWidget::on_moveUpTB_clicked() { outline(LFUN_OUTLINE_UP); diff --git a/src/frontends/qt4/TocWidget.h b/src/frontends/qt4/TocWidget.h index 3f67606302..9cb21bfcbf 100644 --- a/src/frontends/qt4/TocWidget.h +++ b/src/frontends/qt4/TocWidget.h @@ -40,6 +40,9 @@ public: void init(QString const & str); /// void doDispatch(Cursor & cur, FuncRequest const & fr); + ///send request to lyx::dispatch with proper guiview handle + ///(if ToC is detached current_view can be different window) + void sendDispatch(FuncRequest fr); /// bool getStatus(Cursor & cur, FuncRequest const & fr, FuncStatus & status) const;