D20868: Support '#page=' fragment to open at page

[Michael Weghorn]

michaelweghorn created this revision.
michaelweghorn added a reviewer: Okular.
Herald added a project: Okular.
Herald added a subscriber: okular-devel.
michaelweghorn requested review of this revision.

REVISION SUMMARY
  Support the '#page=' fragment to specify which page
  to open the document at, in addition to the existing '#'
  syntax.
  For PDF, the '#page=' fragment is specified in RFC 8118,
  section 3.
  
  BUG: 406831

TEST PLAN
  1. Put a test file that has multiple pages at '/tmp/testfile.pdf'
  2. Check that all of the following commands open the test file on page 2:
  
okular /tmp/testfile.pdf#2 okular /tmp/testfile.pdf#page=2 okular 
file:///tmp/testfile.pdf#2 okular file:///tmp/testfile.pdf#page=2

REPOSITORY
  R223 Okular

BRANCH
  michaelweghorn/kdebug406831_support_rfc_compliant_way_to_open_at_page

REVISION DETAIL
  https://phabricator.kde.org/D20868

AFFECTED FILES
  part.cpp

To: michaelweghorn, #okular
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D20868: Support '#page=' fragment to open at page

[David Hurka]

davidhurka added a comment.


  How about the following approach (not tested)? This way, other parameters 
(like `nameddest`) wouldn’t break it and can be added later.
  

const QString fragment = url.fragment(QUrl::EncodeDelimiters);

bool gotOpenPageNumber = false;
int openPageNumber = fragment.toInt(&gotOpenPageNumber);

for ( QString parameter : fragment.split(QChar('&') )
{
if ( parameter.startsWith("page=", Qt::CaseInsensitive) )
{
bool ok = false;
int page = parameter.mid(5).toInt(&ok);
if ( ok )
{
openPageNumber = page;
gotOpenPageNumber = true;
}
}
}
  
  I`m not sure whether QUrl::EncodeDelimiters is optimal.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D20868

To: michaelweghorn, #okular
Cc: davidhurka, okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D20868: Support '#page=' fragment to open at page

[Albert Astals Cid]

aacid added a comment.


  needs autotest

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D20868

To: michaelweghorn, #okular
Cc: aacid, davidhurka, okular-devel, joaonetto, tfella, ngraham, darcyshen

D20868: Support '#page=' fragment to open at page

[Albert Astals Cid]

aacid requested changes to this revision.
aacid added a comment.
This revision now requires changes to proceed.


  Please move as a Merge Request in https://invent.kde.org/kde/okular
  
  We have pre-commit CI and lots of checks including clazy and clang-tidy there 
so it's a much better place for doing the review/approval/merge of the code.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D20868

To: michaelweghorn, #okular, aacid
Cc: aacid, davidhurka, okular-devel, johnzh, andisa, siddharthmanthan, 
maguirre, fbampaloukas, joaonetto, kezik, tfella, ngraham, darcyshen

D20868: Support '#page=' fragment to open at page

[Michael Weghorn]

michaelweghorn abandoned this revision.
michaelweghorn added a comment.


  In D20868#615357 , @aacid wrote:
  
  > Please move as a Merge Request in https://invent.kde.org/kde/okular
  >
  > We have pre-commit CI and lots of checks including clazy and clang-tidy 
there so it's a much better place for doing the review/approval/merge of the 
code.
  
  
  Finally moved there: 
https://invent.kde.org/graphics/okular/-/merge_requests/265
  Sorry for the extreme delay!

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D20868

To: michaelweghorn, #okular, aacid
Cc: aacid, davidhurka, okular-devel, johnzh, andisa, siddharthmanthan, 
maguirre, fbampaloukas, joaonetto, kezik, ngraham, darcyshen