How to get proper position for createStandardContextMenu for actions that are sensitive to the position where the user clicked inside customContextMenuRequested handler?

Christian Ehrlicher

viewport() also has mapToGlobal() since it's a widget too.

Rian Firth

@Christian-Ehrlicher non of them works for me:

auto viewport = ui->textBrowser->viewport();
qDebug() << viewport->mapToGlobal(pos);
qDebug() << viewport->mapFromGlobal(pos);
qDebug() << viewport->mapToParent(pos);
qDebug() << viewport->mapFromParent(pos);
qDebug() << viewport->mapTo(ui->textBrowser, pos);
qDebug() << viewport->mapFrom(ui->textBrowser, pos);

Clicking in the beginning in approximately top left corner:

QPoint(296,216)
QPoint(-290,-212)
QPoint(4,3)
QPoint(2,1)
QPoint(4,3)
QPoint(2,1)

The same but after scrolling to the bottom:

QPoint(295,217)
QPoint(-291,-211)
QPoint(3,4)
QPoint(1,2)
QPoint(3,4)
QPoint(1,2)

Maybe I'm doing something wrong?

Christian Ehrlicher

@Rian-Firth said in How to get proper position for createStandardContextMenu for actions that are sensitive to the position where the user clicked inside customContextMenuRequested handler?:

qDebug() << viewport->mapToGlobal(pos);

That's the correct one.

Rian Firth

@Christian-Ehrlicher viewport->mapToGlobal(pos) returns screen position [0-1366; 0-768] but createStandardContextMenu(const QPoint &pos) takes local position of scrollarea [0-areaWidth; 0-areaHeight].

As I already said, for example, if viewport has [500; 200] size but the whole document in QTextBrowser (scrollarea itself) is way more in size [500; 2000], vertical scrollbar will be created and createStandardContextMenu(const QPoint &pos) have to take coordinates [500; 2000] but not [500; 200] or [1366; 768].

Christian Ehrlicher

Please post some code where we can see what you're really doing. From your post above you see that viewport->mapToGlobal(pos); returns the same screen coordinates no matter if you scroll or not. Now you're telling me it does not work - this does not match.

Rian Firth

@Christian-Ehrlicher I don't understand. I've already posted the code.
That's how literally it is now:

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
    , ui(new Ui::MainWindow)
{
    ui->setupUi(this);
    ui->textBrowser->setContextMenuPolicy(Qt::CustomContextMenu);

    connect(ui->textBrowser, &QTextBrowser::customContextMenuRequested, this, &MainWindow::OnShowContextMenu);
}

MainWindow::~MainWindow()
{
    delete ui;
}

void MainWindow::OnShowContextMenu(const QPoint& pos)
{
    QMenu* menu = ui->textBrowser->createStandardContextMenu(pos);
    QAction* defaultCopyLink = menu->actions()[1];

    auto viewport = ui->textBrowser->viewport();
    qDebug() << viewport->mapToGlobal(pos);
    qDebug() << viewport->mapFromGlobal(pos);
    qDebug() << viewport->mapToParent(pos);
    qDebug() << viewport->mapFromParent(pos);
    qDebug() << viewport->mapTo(ui->textBrowser, pos);
    qDebug() << viewport->mapFrom(ui->textBrowser, pos);

    connect(defaultCopyLink, &QAction::triggered, [](){
        auto clipboard = QApplication::clipboard();
        clipboard->setText("prefix." + clipboard->text());
    });

    menu->exec(ui->textBrowser->mapToGlobal(pos));
    delete menu;
}

Yes, I see that viewport->mapToGlobal(pos); returns the same screen coordinates no matter if I scroll or not. And it makes sense because it's screen coordinates, it's not related to scroll area anyhow. But I don't need screen coordinates. createStandardContextMenu doesn't take screen position.
Passing screen position from viewport->mapToGlobal(pos); to createStandardContextMenu doesn't work. Actions that are sensitive to the position where the user clicked ("Copy Link Location") won't work like that.

Sorry if I'm confusing you.
Please, tell me if you need anything else, I'll try my best.

Rian Firth

@Christian-Ehrlicher I created handler for customContextMenuRequested signal so I could create my own QMenu for QTextBrowser:

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
    , ui(new Ui::MainWindow)
{
    ui->setupUi(this);

    ui->textBrowser->setContextMenuPolicy(Qt::CustomContextMenu);
    connect(ui->textBrowser, &QTextBrowser::customContextMenuRequested, this, &MainWindow::OnShowContextMenu);
}

void MainWindow::OnShowContextMenu(const QPoint& pos)
{
    QMenu* menu = ui->textBrowser->createStandardContextMenu(pos);
    menu->exec(ui->textBrowser->mapToGlobal(pos));
    delete menu;
}

createStandardContextMenu just creates standard menu for QTextBrowser:

Copy
Copy Link Location
------------------
Select All

But this code doesn't work properly because I'm passing pos from OnShowContextMenu(const QPoint& pos) function like this createStandardContextMenu(pos). So "Copy Link Location" action from menu doesn't work when I'm trying to right click on any link in QTextBrowser when I scrolled down.

The problem is that:

The position pos is the position of the context menu event that the widget receives. Normally this is in widget coordinates. The exception to this rule is QAbstractScrollArea and its subclasses that map the context menu event to coordinates of the viewport().

QTextBrowser is a subclass of QAbstractScrollArea so pos is just in viewport() coordinates which is [0-vpWidth;0-vpHeight].
But createStandardContextMenu needs coordinates [0-scrollAreaWidth;0-scrollAreaHeight]. I might be calling it wrong but by scrollArea I mean the whole document that is in QTextBrowser.

mrjj

Hi
just call createStandardContextMenu with the translated pos :)

void MainWindow::OnShowContextMenu(const QPoint& pos)
{
    QPoint globalPos = ui->textBrowser->mapToGlobal(pos);
    QMenu* menu = ui->textBrowser->createStandardContextMenu(globalPos); <<<<
    QAction* defaultCopyLink = menu->actions()[1];

    auto viewport = ui->textBrowser->viewport();
    qDebug() << viewport->mapToGlobal(pos);
    qDebug() << viewport->mapFromGlobal(pos);
    qDebug() << viewport->mapToParent(pos);
    qDebug() << viewport->mapFromParent(pos);
    qDebug() << viewport->mapTo(ui->textBrowser, pos);
    qDebug() << viewport->mapFrom(ui->textBrowser, pos);

    connect(defaultCopyLink, &QAction::triggered, [](){
        auto clipboard = QApplication::clipboard();
        clipboard->setText("prefix." + clipboard->text());
    });

    menu->exec(globalPos);
    delete menu;

alt text

Rian Firth

@mrjj I just literally copied your example but it doesn't work for me even when I don't scroll down. :c

mrjj

@Rian-Firth
Hi
I just used your code.
I did check it calls your QAction::triggered lambda and all seems ok.

This is Qt 5.15.1 on windows.

And we are 100% sure those are actual links ?
If you click them page is changed ?

Rian Firth

@mrjj it's not about QAction::triggered lambda. It's about that my "Copy Link Location" is not even highlighted when I right click on any link. So I can't even press "Copy Link Location". Basically it has the same behavior when you press in any empty space: you can't press it and it's gray (not highlighted) as if it's disabled. And it's like this even if you click on links.

They're 100% actual links and if I click on them page is changed (becomes empty).

mrjj

@Rian-Firth
Hi
I understand the issue that Copy Link is not enabled. ( so it can't be clicked)

Ok so could be a platform thing as it clearly works here.

What Qt version and platform are you on?

Would it be possible to zip your project and paste link here ?
I could then run it and if that still dont work its something else that Qt version/ platform.

Christian Ehrlicher

@mrjj said in How to get proper position for createStandardContextMenu for actions that are sensitive to the position where the user clicked inside customContextMenuRequested handler?:

createStandardContextMenu

this function needs local coordinates: "This function creates the standard context menu which is shown when the user clicks on the text edit with the right mouse button. It is called from the default contextMenuEvent() handler and it takes the position in document coordinates where the mouse click was. This can enable actions that are sensitive to the position where the user clicked. The popup menu's ownership is transferred to the caller."

Rian Firth

@mrjj I've tried it on 5.12.9 and 5.15.1, Windows 10.
Here you are: https://mega.nz/file/Ik8x1SZS#I6UpE-M-1nWurI1KgUN6esbgcYdpfxh88rPTkLVbUZM

Christian Ehrlicher


void MainWindow::OnShowContextMenu(const QPoint& pos)
{
    QMenu* menu = ui->textBrowser->createStandardContextMenu(pos);

'Normally this is in widget coordinates'
...
'and it takes the position in document coordinates where the mouse click was'

Rian Firth

@Christian-Ehrlicher true. How to get that position in document coordinates considering scrolling tho.

Christian Ehrlicher

Ok, now I've a problem or we found a bug. Looks like createStandardContextMenu(QPoint) is not adding the viewport offset. anchorAt() and cursorForPosition() are doing right.
Here the working code (only adjusted y, x must be done the same)

void MainWindow::OnShowContextMenu(const QPoint& pos)
{
    QPoint p = pos;
    p.ry() += ui->textBrowser->verticalScrollBar()->value();
    qDebug() << p;
    QMenu* menu = ui->textBrowser->createStandardContextMenu(p);

/edit: and no bug report for this... strange
/edit2: Using actions()[1] is not good - better search the object with menu->qFindChild<QAction*>("link-copy")
/edit3: at least it was known (also not fixed/explained how to fix it by yourself): https://codereview.qt-project.org/c/qt/qtbase/+/105486
/edit4: https://bugreports.qt.io/browse/QTBUG-89439

Rian Firth

@Christian-Ehrlicher thank you so much for workaround, menu->qFindChild<QAction*>("link-copy") advise and bug report!