Bug 407884 - Previous/Next Bookmark don’t work like their section title suggests if triggered from Contents side panel context menu
Summary: Previous/Next Bookmark don’t work like their section title suggests if trigge...
Status: REPORTED
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: 1.7.1
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-23 23:32 UTC by Laura David Hurka
Modified: 2019-05-24 22:37 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Laura David Hurka 2019-05-23 23:32:07 UTC
SUMMARY
Just discovered that the context menu in the Contents side panel offers the two actions “Previous Bookmark” and “Next Bookmark”.
They are put in a section named "Page N", where N is the *number* of the page to which the current item in the Contents tree view points.
Nevertheless, these two actions use the current viewport for determining the next/previous bookmark, not the page N like the section title suggests.

STEPS TO REPRODUCE
1. Open a document with many pages and TOC
2. Create some bookmarks roughly at 1/3 and 2/3 of the document
3. Go to the middle of the document
4. Open the Contents panel
5. Open the context menu on the first item.
6. The section “Page 1” contains the action “Next Bookmark”

OBSERVED RESULT
Okular::Part::m_nextBookmark is triggered and brings me to the bookmark at 2/3 of the document.

EXPECTED RESULT
The next bookmark from page 1 is at 1/3 of the document, so it brings me there.
I expect the action to respect the page number in the section title, because the other actions in the section (“Add Bookmark”) also do.

SOFTWARE/OS VERSIONS
KDE Neon 5.15.5
KDE Plasma 5.58.0
Qt 5.12.0
Okular 1.7.1

ADDITIONAL INFORMATION
The context menu ist built in Okular::Part::slotShowTOCMenu(), which calls Okular::Part::showMenu().
This adds the actions “Add Bookmark” / “Remove Bookmark” and “Fit Width” to the context menu.
These actions are custom made for page N.
However, it also adds m_prevBookmark and m_nextBookmark, which don’t know anything about page N. When triggered, they just fetch the current viewport.

ADDITONAL ADDITIONAL INFORMATION
In the summary, I wrote: "Page N", where N is the *number* of the page.
However, the Contents tree view items show the *name* of the page. This may be confusing, because the user might see two different numbers in the tree view item and the context menu.
Comment 1 Albert Astals Cid 2019-05-24 22:24:14 UTC
the fact that they don't do what you want doesn't mean they don't work correctly.
Comment 2 Laura David Hurka 2019-05-24 22:28:11 UTC
So it’s intended that they don’t do what the menu says, but do what the menu would say if invoked another way?
Comment 3 Albert Astals Cid 2019-05-24 22:33:31 UTC
yes, they do what they were coded to do, so it is intended they do that.

You get confused by the heading, fair enough, my guess is that orignally there was no heading. It's basically the same menu that you get on the main page area and thus behaves the same.
Comment 4 Laura David Hurka 2019-05-24 22:37:20 UTC
Ok, I renamed the bug.

IMHO they shouldn’t be in a context menu if they don’t respect the context of the menu.