Commit 96cda154 authored by Tomaz  Canabrava's avatar Tomaz Canabrava Committed by Kurt Hindenburg
Browse files

Fix context menu actions when search bar is enabled

The actions are destroyed during a focusIn / focusOut, and recreated
with the same content but different memory positions, and that
killed the QActions inside of the menu.

To trigger this is simple, open the search bar, type some url in
konsole, and right click on it: you will not see the actions
to copy and go to url.

moving the related code to QSharedPointer fixes this by delaying
the destruction of the pointer untill the menu is closed.
parent 9b51ec79
......@@ -92,12 +92,12 @@ void FilterChain::clear()
QList<Filter *>::clear();
}
Filter::HotSpot *FilterChain::hotSpotAt(int line, int column) const
QSharedPointer<Filter::HotSpot> FilterChain::hotSpotAt(int line, int column) const
{
QListIterator<Filter *> iter(*this);
while (iter.hasNext()) {
Filter *filter = iter.next();
Filter::HotSpot *spot = filter->hotSpotAt(line, column);
QSharedPointer<Filter::HotSpot> spot = filter->hotSpotAt(line, column);
if (spot != nullptr) {
return spot;
}
......@@ -106,9 +106,9 @@ Filter::HotSpot *FilterChain::hotSpotAt(int line, int column) const
return nullptr;
}
QList<Filter::HotSpot *> FilterChain::hotSpots() const
QList<QSharedPointer<Filter::HotSpot>> FilterChain::hotSpots() const
{
QList<Filter::HotSpot *> list;
QList<QSharedPointer<Filter::HotSpot>> list;
QListIterator<Filter *> iter(*this);
while (iter.hasNext()) {
Filter *filter = iter.next();
......@@ -180,8 +180,6 @@ void TerminalImageFilterChain::setImage(const Character * const image, int lines
}
Filter::Filter() :
_hotspots(QMultiHash<int, HotSpot *>()),
_hotspotList(QList<HotSpot *>()),
_linePositions(nullptr),
_buffer(nullptr)
{
......@@ -195,10 +193,6 @@ Filter::~Filter()
void Filter::reset()
{
_hotspots.clear();
QListIterator<HotSpot *> iter(_hotspotList);
while (iter.hasNext()) {
delete iter.next();
}
_hotspotList.clear();
}
......@@ -238,7 +232,7 @@ const QString *Filter::buffer()
Filter::HotSpot::~HotSpot() = default;
void Filter::addHotSpot(HotSpot *spot)
void Filter::addHotSpot(QSharedPointer<HotSpot> spot)
{
_hotspotList << spot;
......@@ -247,16 +241,16 @@ void Filter::addHotSpot(HotSpot *spot)
}
}
QList<Filter::HotSpot *> Filter::hotSpots() const
QList<QSharedPointer<Filter::HotSpot>> Filter::hotSpots() const
{
return _hotspotList;
}
Filter::HotSpot *Filter::hotSpotAt(int line, int column) const
QSharedPointer<Filter::HotSpot> Filter::hotSpotAt(int line, int column) const
{
const QList<HotSpot *> hotspots = _hotspots.values(line);
const auto hotspots = _hotspots.values(line);
for (HotSpot *spot : hotspots) {
for (auto &spot : hotspots) {
if (spot->startLine() == line && spot->startColumn() > column) {
continue;
}
......@@ -369,8 +363,8 @@ void RegExpFilter::process()
getLineColumn(match.capturedStart(), startLine, startColumn);
getLineColumn(match.capturedEnd(), endLine, endColumn);
RegExpFilter::HotSpot *spot = newHotSpot(startLine, startColumn,
endLine, endColumn, match.capturedTexts());
QSharedPointer<Filter::HotSpot> spot(newHotSpot(startLine, startColumn,
endLine, endColumn, match.capturedTexts()));
if (spot == nullptr) {
continue;
}
......@@ -379,24 +373,23 @@ void RegExpFilter::process()
}
}
RegExpFilter::HotSpot *RegExpFilter::newHotSpot(int startLine, int startColumn, int endLine,
QSharedPointer<Filter::HotSpot> RegExpFilter::newHotSpot(int startLine, int startColumn, int endLine,
int endColumn, const QStringList &capturedTexts)
{
return new RegExpFilter::HotSpot(startLine, startColumn,
endLine, endColumn, capturedTexts);
return QSharedPointer<Filter::HotSpot>(new RegExpFilter::HotSpot(startLine, startColumn,
endLine, endColumn, capturedTexts));
}
RegExpFilter::HotSpot *UrlFilter::newHotSpot(int startLine, int startColumn, int endLine,
QSharedPointer<Filter::HotSpot> UrlFilter::newHotSpot(int startLine, int startColumn, int endLine,
int endColumn, const QStringList &capturedTexts)
{
return new UrlFilter::HotSpot(startLine, startColumn,
endLine, endColumn, capturedTexts);
return QSharedPointer<Filter::HotSpot>(new UrlFilter::HotSpot(startLine, startColumn,
endLine, endColumn, capturedTexts));
}
UrlFilter::HotSpot::HotSpot(int startLine, int startColumn, int endLine, int endColumn,
const QStringList &capturedTexts) :
RegExpFilter::HotSpot(startLine, startColumn, endLine, endColumn, capturedTexts),
_urlObject(new FilterObject(this))
RegExpFilter::HotSpot(startLine, startColumn, endLine, endColumn, capturedTexts)
{
setType(Link);
}
......@@ -469,18 +462,12 @@ UrlFilter::UrlFilter()
UrlFilter::HotSpot::~HotSpot()
{
delete _urlObject;
}
void FilterObject::activated()
{
_filter->activate(sender());
}
QList<QAction *> UrlFilter::HotSpot::actions()
{
auto openAction = new QAction(_urlObject);
auto copyAction = new QAction(_urlObject);
auto openAction = new QAction(this);
auto copyAction = new QAction(this);
const UrlType kind = urlType();
Q_ASSERT(kind == StandardUrl || kind == Email);
......@@ -503,16 +490,10 @@ QList<QAction *> UrlFilter::HotSpot::actions()
openAction->setObjectName(QStringLiteral("open-action"));
copyAction->setObjectName(QStringLiteral("copy-action"));
QObject::connect(openAction, &QAction::triggered, _urlObject,
&Konsole::FilterObject::activated);
QObject::connect(copyAction, &QAction::triggered, _urlObject,
&Konsole::FilterObject::activated);
QList<QAction *> actions;
actions << openAction;
actions << copyAction;
QObject::connect(openAction, &QAction::triggered, this, [this, openAction]{ activate(openAction); });
QObject::connect(copyAction, &QAction::triggered, this, [this, copyAction]{ activate(copyAction); });
return actions;
return {openAction, copyAction};
}
/**
......@@ -522,7 +503,7 @@ QList<QAction *> UrlFilter::HotSpot::actions()
* https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_267
*/
RegExpFilter::HotSpot *FileFilter::newHotSpot(int startLine, int startColumn, int endLine,
QSharedPointer<Filter::HotSpot> FileFilter::newHotSpot(int startLine, int startColumn, int endLine,
int endColumn, const QStringList &capturedTexts)
{
if (_session.isNull()) {
......@@ -540,7 +521,7 @@ RegExpFilter::HotSpot *FileFilter::newHotSpot(int startLine, int startColumn, in
return nullptr;
}
return new FileFilter::HotSpot(startLine, startColumn, endLine, endColumn, capturedTexts, _dirPath + filename);
return QSharedPointer<Filter::HotSpot>(new FileFilter::HotSpot(startLine, startColumn, endLine, endColumn, capturedTexts, _dirPath + filename));
}
void FileFilter::process()
......@@ -555,7 +536,6 @@ void FileFilter::process()
FileFilter::HotSpot::HotSpot(int startLine, int startColumn, int endLine, int endColumn,
const QStringList &capturedTexts, const QString &filePath) :
RegExpFilter::HotSpot(startLine, startColumn, endLine, endColumn, capturedTexts),
_fileObject(new FilterObject(this)),
_filePath(filePath)
{
setType(Link);
......@@ -626,16 +606,12 @@ FileFilter::FileFilter(Session *session) :
FileFilter::HotSpot::~HotSpot()
{
delete _fileObject;
}
QList<QAction *> FileFilter::HotSpot::actions()
{
auto openAction = new QAction(_fileObject);
auto openAction = new QAction(this);
openAction->setText(i18n("Open File"));
QObject::connect(openAction, &QAction::triggered, _fileObject,
&Konsole::FilterObject::activated);
QList<QAction *> actions;
actions << openAction;
return actions;
QObject::connect(openAction, &QAction::triggered, this, [this ]{ activate(); });
return {openAction};
}
......@@ -32,6 +32,9 @@
// Konsole
#include "Character.h"
// std
#include <optional>
class QAction;
namespace Konsole {
......@@ -70,7 +73,7 @@ public:
* Hotspots may have more than one action, in which case the list of actions can be obtained using the
* actions() method. These actions may then be displayed in a popup menu or toolbar for example.
*/
class HotSpot
class HotSpot : public QObject
{
public:
/**
......@@ -143,10 +146,10 @@ public:
void reset();
/** Returns the hotspot which covers the given @p line and @p column, or 0 if no hotspot covers that area */
HotSpot *hotSpotAt(int line, int column) const;
QSharedPointer<HotSpot> hotSpotAt(int line, int column) const;
/** Returns the list of hotspots identified by the filter */
QList<HotSpot *> hotSpots() const;
QList<QSharedPointer<HotSpot>> hotSpots() const;
/** Returns the list of hotspots identified by the filter which occur on a given line */
......@@ -157,7 +160,7 @@ public:
protected:
/** Adds a new hotspot to the list */
void addHotSpot(HotSpot *);
void addHotSpot(QSharedPointer<HotSpot> spot);
/** Returns the internal buffer */
const QString *buffer();
/** Converts a character position within buffer() to a line and column */
......@@ -166,8 +169,8 @@ protected:
private:
Q_DISABLE_COPY(Filter)
QMultiHash<int, HotSpot *> _hotspots;
QList<HotSpot *> _hotspotList;
QMultiHash<int, QSharedPointer<HotSpot>> _hotspots;
QList<QSharedPointer<HotSpot>> _hotspotList;
const QList<int> *_linePositions;
const QString *_buffer;
......@@ -226,15 +229,13 @@ protected:
* Called when a match for the regular expression is encountered. Subclasses should reimplement this
* to return custom hotspot types
*/
virtual RegExpFilter::HotSpot *newHotSpot(int startLine, int startColumn, int endLine,
virtual QSharedPointer<Filter::HotSpot> newHotSpot(int startLine, int startColumn, int endLine,
int endColumn, const QStringList &capturedTexts);
private:
QRegularExpression _searchText;
};
class FilterObject;
/** A filter which matches URLs in blocks of text */
class UrlFilter : public RegExpFilter
{
......@@ -265,14 +266,12 @@ public:
Unknown
};
UrlType urlType() const;
FilterObject *_urlObject;
};
UrlFilter();
protected:
RegExpFilter::HotSpot *newHotSpot(int, int, int, int, const QStringList &) override;
QSharedPointer<Filter::HotSpot> newHotSpot(int, int, int, int, const QStringList &) override;
private:
static const QRegularExpression FullUrlRegExp;
......@@ -307,7 +306,6 @@ public:
void activate(QObject *object = nullptr) override;
private:
FilterObject *_fileObject;
QString _filePath;
};
......@@ -316,7 +314,7 @@ public:
void process() override;
protected:
RegExpFilter::HotSpot *newHotSpot(int, int, int, int, const QStringList &) override;
QSharedPointer<Filter::HotSpot> newHotSpot(int, int, int, int, const QStringList &) override;
private:
QPointer<Session> _session;
......@@ -324,22 +322,6 @@ private:
QSet<QString> _currentFiles;
};
class FilterObject : public QObject
{
Q_OBJECT
public:
explicit FilterObject(Filter::HotSpot *filter) : _filter(filter)
{
}
public Q_SLOTS:
void activated();
private:
Q_DISABLE_COPY(FilterObject)
Filter::HotSpot *_filter;
};
/**
* A chain which allows a group of filters to be processed as one.
* The chain owns the filters added to it and deletes them when the chain itself is destroyed.
......@@ -380,9 +362,9 @@ public:
void setBuffer(const QString *buffer, const QList<int> *linePositions);
/** Returns the first hotspot which occurs at @p line, @p column or 0 if no hotspot was found */
Filter::HotSpot *hotSpotAt(int line, int column) const;
QSharedPointer<Filter::HotSpot> hotSpotAt(int line, int column) const;
/** Returns a list of all the hotspots in all the chain's filters */
QList<Filter::HotSpot *> hotSpots() const;
QList<QSharedPointer<Filter::HotSpot>> hotSpots() const;
};
/** A filter chain which processes character images from terminal displays */
......
......@@ -1685,11 +1685,12 @@ void SessionController::showDisplayContextMenu(const QPoint& position)
updateReadOnlyActionStates();
// prepend content-specific actions such as "Open Link", "Copy Email Address" etc.
QList<QAction*> contentActions = _view->filterActions(position);
QSharedPointer<Filter::HotSpot> hotSpot = _view->filterActions(position);
if (hotSpot) {
popup->insertActions(popup->actions().value(0, nullptr), hotSpot->actions());
}
auto contentSeparator = new QAction(popup);
contentSeparator->setSeparator(true);
contentActions << contentSeparator;
popup->insertActions(popup->actions().value(0, nullptr), contentActions);
// always update this submenu before showing the context menu,
// because the available search services might have changed
......@@ -1706,7 +1707,9 @@ void SessionController::showDisplayContextMenu(const QPoint& position)
}
}
QAction* chosen = popup->exec(_view->mapToGlobal(position));
// they are here.
// qDebug() << popup->actions().indexOf(contentActions[0]) << popup->actions().indexOf(contentActions[1]) << popup->actions()[3];
QAction* chosen = popup->exec(QCursor::pos());
// check for validity of the pointer to the popup menu
if (!popup.isNull()) {
......
......@@ -1002,7 +1002,7 @@ void TerminalDisplay::scrollImage(int lines , const QRect& screenWindowRegion)
QRegion TerminalDisplay::hotSpotRegion() const
{
QRegion region;
for (const Filter::HotSpot *hotSpot : _filterChain->hotSpots()) {
for (const auto &hotSpot : _filterChain->hotSpots()) {
QRect r;
r.setLeft(hotSpot->startColumn());
r.setTop(hotSpot->startLine());
......@@ -1032,6 +1032,7 @@ QRegion TerminalDisplay::hotSpotRegion() const
void TerminalDisplay::processFilters()
{
if (_screenWindow.isNull()) {
return;
}
......@@ -1390,7 +1391,7 @@ void TerminalDisplay::paintFilters(QPainter& painter)
// iterate over hotspots identified by the display's currently active filters
// and draw appropriate visuals to indicate the presence of the hotspot
const QList<Filter::HotSpot*> spots = _filterChain->hotSpots();
const auto spots = _filterChain->hotSpots();
int urlNumber, urlNumInc;
if (_reverseUrlHints) {
urlNumber = spots.size() + 1;
......@@ -1399,7 +1400,7 @@ void TerminalDisplay::paintFilters(QPainter& painter)
urlNumber = 0;
urlNumInc = 1;
}
for (const Filter::HotSpot *spot : spots) {
for (const auto &spot : spots) {
urlNumber += urlNumInc;
QRegion region;
......@@ -2217,7 +2218,7 @@ void TerminalDisplay::mousePressEvent(QMouseEvent* ev)
}
if ((_openLinksByDirectClick || ((ev->modifiers() & Qt::ControlModifier) != 0u))) {
Filter::HotSpot* spot = _filterChain->hotSpotAt(charLine, charColumn);
auto spot = _filterChain->hotSpotAt(charLine, charColumn);
if ((spot != nullptr) && spot->type() == Filter::HotSpot::Link) {
QObject action;
action.setObjectName(QStringLiteral("open-action"));
......@@ -2238,14 +2239,12 @@ void TerminalDisplay::mousePressEvent(QMouseEvent* ev)
}
}
QList<QAction*> TerminalDisplay::filterActions(const QPoint& position)
QSharedPointer<Filter::HotSpot> TerminalDisplay::filterActions(const QPoint& position)
{
int charLine, charColumn;
getCharacterPosition(position, charLine, charColumn, false);
Filter::HotSpot* spot = _filterChain->hotSpotAt(charLine, charColumn);
return spot != nullptr ? spot->actions() : QList<QAction*>();
return _filterChain->hotSpotAt(charLine, charColumn);
}
void TerminalDisplay::mouseMoveEvent(QMouseEvent* ev)
......@@ -2257,7 +2256,7 @@ void TerminalDisplay::mouseMoveEvent(QMouseEvent* ev)
processFilters();
// handle filters
// change link hot-spot appearance on mouse-over
Filter::HotSpot* spot = _filterChain->hotSpotAt(charLine, charColumn);
auto spot = _filterChain->hotSpotAt(charLine, charColumn);
if ((spot != nullptr) && spot->type() == Filter::HotSpot::Link) {
QRegion previousHotspotArea = _mouseOverHotspotArea;
_mouseOverHotspotArea = QRegion();
......
......@@ -35,6 +35,7 @@
#include "ScrollState.h"
#include "Profile.h"
#include "TerminalHeaderBar.h"
#include "Filter.h"
class QDrag;
class QDragEnterEvent;
......@@ -149,7 +150,7 @@ public:
* Returns a list of menu actions created by the filters for the content
* at the given @p position.
*/
QList<QAction *> filterActions(const QPoint &position);
QSharedPointer<Filter::HotSpot> filterActions(const QPoint &position);
/** Specifies whether or not the cursor can blink. */
void setBlinkingCursorEnabled(bool blink);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment