This patch mostly solves the problem reported in https://bugs.kde.org/show_bug.cgi?id=388532. Though, the popup note is not raised when clicking on the border of the annotation window, or in the latex rederer message.

Can confirm, this definitely improves things with overlapping notes.

Do you feel like you can try adding an autotest for this?

ping?

I've never written an autotest, so I need to figure out how to do it, but I can try. It's gonna take some time, because I'm busy in the next 1-2 weeks.

In D10792#225334, @simgunz wrote:

I've never written an autotest, so I need to figure out how to do it, but I can try. It's gonna take some time, because I'm busy in the next 1-2 weeks.

We have lots of them in autotests/ make sure you have a look, you probably don't even need a new one, just add a new test function to an existing test file.

I'm adding a new test to parttest.cpp because is the only test where there is the part component that allows me to access the m_pageView.

I'm trying to call the public qslot:

part.m_pageView->openAnnotationWindow(annot1, 0);

but I get this error:

parttest.cpp:(.text+0x14d8d): undefined reference to PageView::openAnnotationWindow(Okular::Annotation*, int)

I've also cleaned the build dir and rerun cmake, and make but it still happens.

Any idea?

Moreover, then I don't know how to access the opened annotation window. The reference is stored in part->d->m_annowindows, in the private class of PageView, which is not accessible from the tester I think.

In D10792#226357, @simgunz wrote:

I'm adding a new test to parttest.cpp because is the only test where there is the part component that allows me to access the m_pageView.

I'm trying to call the public qslot:

part.m_pageView->openAnnotationWindow(annot1, 0);

but I get this error:

parttest.cpp:(.text+0x14d8d): undefined reference to PageView::openAnnotationWindow(Okular::Annotation*, int)

I've also cleaned the build dir and rerun cmake, and make but it still happens.

Any idea?

Yes, pageview is not an exported class so you can't really call it's methods because "they don't exist" for the linker.

You have three posibilities:

• Export the class, easiest, but i'd really like to avoid that
• Since openAnnotationWindow is a slot, you should be able to use QMetaObject::invokeMethod to call it
• instead of calling the method, just do what the user would do, i.e. simulate a click/double click in the correct places

Moreover, then I don't know how to access the opened annotation window. The reference is stored in part->d->m_annowindows, in the private class of PageView, which is not accessible from the tester I think.

Right, but you can use QObject::findChild[ren] to look for AnnotWindow childs of m_pageView, that should be enough to get the pointer to the AnnotWindow.

I've progressed a bit on this, but now I get the following error:

/usr/include/qt/QtCore/qobject.h:179: undefined reference to `AnnotWindow::staticMetaObject'

executing this line:

AnnotWindow *wdw = part.m_pageView->findChild<AnnotWindow *>();

I have added #include "../ui/annotwindow.h" and in the CMake file parttest is linked with okularpart which included annotwindow.cpp

I also reconfigured CMake.

Any help?

Ok. I figured it out myself.

I need to export AnnotWindow (add OKULARCORE_EXPORT to the class AnnotWindow) if I want to use findChild to retrieve it. I am not sure if this should be exported just for the autotest. Maybe there is an alternative method to avoid this.

I have few questions regarding the internal structure of okular and how to write autotests:

1. To click on an annotation to open the annotation window I though to do the following (partially copied from other tests):

const int width =  part.m_pageView->horizontalScrollBar()->maximum() +
                       part.m_pageView->viewport()->width();
const int height = part.m_pageView->verticalScrollBar()->maximum() +
                       part.m_pageView->viewport()->height();

NormalizedPoint pt = annot1->boundingRectangle().center();

QTest::mouseDClick(part.m_pageView->viewport(), Qt::LeftButton, Qt::NoModifier, QPoint(width * pt.x,  height * pt.y));

but in this way I don't correctly click on the annotation. For now I manually set the QPoint to hit correctly the annotation.

I don't understand what the normalized area where a NormalizedPoint can be corresponds to. Can someone explain me better this mechanics?

Why is width equal to the maximum value of the scrollbar plus the viewport?

2. When I run an autotest I saw a window appearing showing the file opened in okular. I cannot interact with this window though. What is the purpose of that? How can I have a visual feedback on the code I am writing in the autotest?

In D10792#237224, @simgunz wrote:

Ok. I figured it out myself.

I need to export AnnotWindow (add OKULARCORE_EXPORT to the class AnnotWindow) if I want to use findChild to retrieve it. I am not sure if this should be exported just for the autotest. Maybe there is an alternative method to avoid this.

Do you need AnnotWindow itself? Probably you're just fine searching/using QFrame (its parent class)? In this case you don't need to export AnnowWindow.

In D10792#237792, @simgunz wrote:

I have few questions regarding the internal structure of okular and how to write autotests:

1. To click on an annotation to open the annotation window I though to do the following (partially copied from other tests):
   
   ` const int width = part.m_pageView->horizontalScrollBar()->maximum() + part.m_pageView->viewport()->width(); const int height = part.m_pageView->verticalScrollBar()->maximum() + part.m_pageView->viewport()->height();
   
   NormalizedPoint pt = annot1->boundingRectangle().center();
   
   QTest::mouseDClick(part.m_pageView->viewport(), Qt::LeftButton, Qt::NoModifier, QPoint(width * pt.x, height * pt.y)); `
   
   but in this way I don't correctly click on the annotation. For now I manually set the QPoint to hit correctly the annotation.
   
   I don't understand what the normalized area where a NormalizedPoint can be corresponds to. Can someone explain me better this mechanics?

On a quick look i feel it should work.

What you don't understand about NormalizedXXX, they are basically a 0 to 1 representation of the position of something on the page

Why is width equal to the maximum value of the scrollbar plus the viewport?

So let's say your content is 1000 wide, and your viewport is 700 wide, what will be the maximum value of your scroll bar? 300

So if you want to know the width of the content you add up 700 and 300

Does this make sense?

2. When I run an autotest I saw a window appearing showing the file opened in okular. I cannot interact with this window though.

Sure you can

What is the purpose of that?

That is the purpose of what?

How can I have a visual feedback on the code I am writing in the autotest?

Not sure i understand the question, you mean you want to look at the window?
Add QTest::qWait(someseonds)

Thanks to your suggestions I made some progresses.

My current autotest looks as in the block below.

Problems:

1. If I do not export AnnotWindow and call QVERIFY( part.m_pageView->findChildren<QFrame *>().size() == 1 ) it fails, while calling QVERIFY( part.m_pageView->findChildren<AnnotWindow *>().size() == 1 ) would succeed.

2. The value returned by annot1->boundingRectangle().center(); is wrong.

Moreover if I I pass to mouseMove a point within the coordinates I specify when I construct the annotation the mouse moves to the wrong position.
In particular the mouse moves to a Y position way below the annotation.
So either I am missing something or there is a bug.

Example where it fails:

annot1->setBoundingRectangle( Okular::NormalizedRect( 0.8, 0.1, 0.85, 0.15 ) );

QTest::mouseMove(part.m_pageView->viewport(), QPoint(width * 0.82,  height * 0.13));

For now I am using values find through trial and error.

3. How do I click on the annotation window?

These two commands both fail

QTest::mouseClick(part.m_pageView->viewport(), Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.1,  height * 0.06)); // The window is under the mouse
QTest::mouseClick(win2, Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.1,  height * 0.06)); // win2 is the annotation window widget (under the mouse again)

Isn't it possible to call mouseClick without specifying the target widget, as if I would click manually so that the top widget is the target of the click?

void PartTest::testAnnotWindow()
{
    QVariantList dummyArgs;
    Okular::Part part(nullptr, nullptr, dummyArgs);
    QVERIFY(openDocument(&part, QStringLiteral(KDESRCDIR "data/file2.pdf")));
    part.widget()->show();
    QVERIFY(QTest::qWaitForWindowExposed(part.widget()));

    const int width =  part.m_pageView->horizontalScrollBar()->maximum() +
                       part.m_pageView->viewport()->width();
    const int height = part.m_pageView->verticalScrollBar()->maximum() +
                       part.m_pageView->viewport()->height();

    part.m_document->setViewportPage(0);

    // wait foqqr pixmap
    QTRY_VERIFY(part.m_document->page(0)->hasPixmap(part.m_pageView));

    QMetaObject::invokeMethod(part.m_pageView, "slotSetMouseNormal");

    QCOMPARE(part.m_document->currentPage(), 0u);


    // Create two distinct text annotations
    Okular::Annotation * annot1 = new Okular::TextAnnotation();
    annot1->setBoundingRectangle( Okular::NormalizedRect( 0.8, 0.1, 0.85, 0.15 ) );
    annot1->setContents( QStringLiteral("Annot contents 111111") );

    Okular::Annotation *annot2 = new Okular::TextAnnotation();
    annot2->setBoundingRectangle( Okular::NormalizedRect(  0.8, 0.3, 0.85, 0.35 ) );
    annot2->setContents( QStringLiteral("Annot contents 222222") );

    int waitDealy = 1000;

    // Add annot1 and annot2 to document
    QTest::qWait(waitDealy);
    part.m_document->addPageAnnotation( 0, annot1 );
    QTest::qWait(waitDealy);
    part.m_document->addPageAnnotation( 0, annot2 );
    QTest::qWait(waitDealy);
    QVERIFY( part.m_document->page( 0 )->annotations().size() == 2 );

    // Double click the first annotation to open its window (move mouse for visual feedback)
    //NormalizedPoint pt = annot1->boundingRectangle().center();
    QTest::mouseMove(part.m_pageView->viewport(), QPoint(width * 0.81,  height * 0.06));
    //QTest::mouseMove(part.m_pageView->viewport(), QPoint(width * pt.x, height * pt.y));
    QTest::qWait(waitDealy);
    QTest::mouseDClick(part.m_pageView->viewport(), Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.81,  height * 0.06));
    //QTest::mouseDClick(part.m_pageView->viewport(), Qt::LeftButton, Qt::NoModifier, QPoint(width * pt.x, height * pt.y));
    QTest::qWait(waitDealy);
    QVERIFY( part.m_pageView->findChildren<AnnotWindow *>().size() == 1 );
    // Verify that the window is visible
    AnnotWindow * win1 = part.m_pageView->findChild<AnnotWindow *>();
    QVERIFY( !win1->visibleRegion().isEmpty() );

    // Double click the second annotation to open its window (move mouse for visual feedback)
    QTest::mouseMove(part.m_pageView->viewport(), QPoint(width * 0.81,  height * 0.16));
    QTest::qWait(waitDealy);
    QTest::mouseDClick(part.m_pageView->viewport(), Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.81,  height * 0.16));
    QTest::qWait(waitDealy);
    QVERIFY( part.m_pageView->findChildren<AnnotWindow *>().size() == 2 );
    // Verify that the first window is hidden covered by the second, which is visible
    QList<AnnotWindow *> lstWin = part.m_pageView->findChildren<AnnotWindow *>();
    QFrame * win2;
    if (lstWin[0] == win1) {
        win2 = lstWin[1];
    } else {
        win2 = lstWin[0];
    }
    QVERIFY( win1->visibleRegion().isEmpty() );
    QVERIFY( !win2->visibleRegion().isEmpty() );

    // Double click the first annotation to raise its window (move mouse for visual feedback)
    QTest::mouseMove(part.m_pageView->viewport(), QPoint(width * 0.81,  height * 0.06));
    QTest::qWait(waitDealy);
    QTest::mouseDClick(part.m_pageView->viewport(), Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.81,  height * 0.06));
    QTest::qWait(waitDealy);
    // Verify that the second window is hidden covered by the first, which is visible
    QVERIFY( !win1->visibleRegion().isEmpty() );
    QVERIFY( win2->visibleRegion().isEmpty() );

    // Move annotation window 1 to partially show annotation window 2
    QTest::qWait(waitDealy);
    win1->move(QPoint(width * 0.15,  height * 0.1));
    QTest::qWait(waitDealy);
    // Verify that both windows are partially visible
    QVERIFY( !win1->visibleRegion().isEmpty() );
    QVERIFY( !win2->visibleRegion().isEmpty() );

    // Double click the first annotation to raise its window (move mouse for visual feedback)
    QTest::mouseMove(part.m_pageView->viewport(), QPoint(width * 0.1,  height * 0.06));
    QTest::qWait(1000);
    QTest::mouseClick(win2, Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.1,  height * 0.06));
    QTest::qWait(5000);
}

In D10792#241405, @simgunz wrote:

Thanks to your suggestions I made some progresses.

My current autotest looks as in the block below.

Problems:

1. If I do not export AnnotWindow and call QVERIFY( part.m_pageView->findChildren<QFrame *>().size() == 1 ) it fails, while calling QVERIFY( part.m_pageView->findChildren<AnnotWindow *>().size() == 1 ) would succeed.
2. The value returned by annot1->boundingRectangle().center(); is wrong. Moreover if I I pass to mouseMove a point within the coordinates I specify when I construct the annotation the mouse moves to the wrong position. In particular the mouse moves to a Y position way below the annotation. So either I am missing something or there is a bug.

There's a "bug" in the test, you're opening a two page file, so the height variable is the height of the two pages, and then all the math fails when trying to find the proper y, changing to open file1.pdf makes it better.

https://paste.kde.org/phoqbec1x "WORKS" in my computer, you still need to add more stuff to the end of the test, right? at least a verify :)

3. How do I click on the annotation window?
   
   These two commands both fail ` QTest::mouseClick(part.m_pageView->viewport(), Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.1, height * 0.06)); The window is under the mouse QTest::mouseClick(win2, Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.1, height * 0.06)); win2 is the annotation window widget (under the mouse again)
   
   `
   
   Isn't it possible to call mouseClick without specifying the target widget, as if I would click manually so that the top widget is the target of the click?

It's a bit tricky, check what i did on the paste. Probably there's a better way to do it, but it's a bit late now so this works for me.

In D10792#241504, @aacid wrote:

In D10792#241405, @simgunz wrote:

Thanks to your suggestions I made some progresses.

My current autotest looks as in the block below.

Problems:

1. If I do not export AnnotWindow and call QVERIFY( part.m_pageView->findChildren<QFrame *>().size() == 1 ) it fails, while calling QVERIFY( part.m_pageView->findChildren<AnnotWindow *>().size() == 1 ) would succeed.
2. The value returned by annot1->boundingRectangle().center(); is wrong. Moreover if I I pass to mouseMove a point within the coordinates I specify when I construct the annotation the mouse moves to the wrong position. In particular the mouse moves to a Y position way below the annotation. So either I am missing something or there is a bug.

There's a "bug" in the test, you're opening a two page file, so the height variable is the height of the two pages, and then all the math fails when trying to find the proper y, changing to open file1.pdf makes it better.

Thanks for the explanation. I'm slowly understanding how this coordinate system works.

https://paste.kde.org/phoqbec1x "WORKS" in my computer, you still need to add more stuff to the end of the test, right? at least a verify :)

Yes. I have added a check on the number of rects composing the visible region, which should be four for a fully visible window ( if I am not wrong it is because the KTextEdit is on top of the parent widget of the AnnotWindow, so that this last one is never fully visible). Actually also for a partially visible window they can be four, I think, but for the particular position where I moved window 1 they are three. I hope this is enough as a test.

3. How do I click on the annotation window?
   
   These two commands both fail ` QTest::mouseClick(part.m_pageView->viewport(), Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.1, height * 0.06)); The window is under the mouse QTest::mouseClick(win2, Qt::LeftButton, Qt::NoModifier, QPoint(width * 0.1, height * 0.06)); win2 is the annotation window widget (under the mouse again)
   
   `
   
   Isn't it possible to call mouseClick without specifying the target widget, as if I would click manually so that the top widget is the target of the click?

It's a bit tricky, check what i did on the paste. Probably there's a better way to do it, but it's a bit late now so this works for me.

Ok. Understood it.

I have left two qWait after adding the annotations because without those the test was failing some times.

When double clicking an already open annotation, we bring it to front, but it doesn't get keyboard focus (the text cursos doesn't blink), which is a different behaviour from when the annocation is closed.

Do you think that is something worth trying to fix here or you consider that a differnet issue and would like to get this merged first?

I'll have a look at it. I think that focusing it makes sense also to locate which popup note is associated with the annotation clicked (especially if they are not overlapped).