D25229: Update docs to the new annotation toolbar

[Nathaniel Graham]

ngraham accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R223 Okular

BRANCH
  master

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

To: yurchor, #okular, ngraham
Cc: ngraham, okular-devel, kde-doc-english, johnzh, andisa, siddharthmanthan, 
maguirre, gennad, fbampaloukas, joaonetto, kezik, tfella, skadinna, darcyshen, 
aacid

D15580: New annotation toolbar

[Nathaniel Graham]

ngraham added a dependent revision: D25229: Update docs to the new annotation 
toolbar.

REPOSITORY
  R223 Okular

BRANCH
  new-annotation-toolbar_ToggleActionMenu

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

To: simgunz, #okular, #vdg, ngraham
Cc: kde-doc-english, andreashurka, yurchor, trickyricky26, simgunz, ltoscano, 
cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, 
johnzh, andisa, siddharthmanthan, maguirre, gennad, fbampaloukas, joaonetto, 
kezik, tfella, skadinna, darcyshen

D25229: Update docs to the new annotation toolbar

[Nathaniel Graham]

ngraham edited the summary of this revision.
ngraham added a dependency: D15580: New annotation toolbar.

REPOSITORY
  R223 Okular

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

To: yurchor, #okular
Cc: ngraham, okular-devel, kde-doc-english, johnzh, andisa, siddharthmanthan, 
maguirre, gennad, fbampaloukas, joaonetto, kezik, tfella, skadinna, darcyshen, 
aacid

[okular] [Bug 413746] Add option to only rotate specific page(s)

[Nate Graham]

https://bugs.kde.org/show_bug.cgi?id=413746

Nate Graham  changed:

   What|Removed |Added

 Blocks|413745  |


Referenced Bugs:

https://bugs.kde.org/show_bug.cgi?id=413745
[Bug 413745] Add shortcuts for left / right page rotation
-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 413745] Add shortcuts for left / right page rotation

[Nate Graham]

https://bugs.kde.org/show_bug.cgi?id=413745

Nate Graham  changed:

   What|Removed |Added

 Depends on|413746  |


Referenced Bugs:

https://bugs.kde.org/show_bug.cgi?id=413746
[Bug 413746] Add option to only rotate specific page(s)
-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 307224] Ability to rotate single pages

[Nate Graham]

https://bugs.kde.org/show_bug.cgi?id=307224

Nate Graham  changed:

   What|Removed |Added

 Status|REPORTED|CONFIRMED
 Ever confirmed|0   |1

-- 
You are receiving this mail because:
You are the assignee for the bug.

D21971: Replace ToolAction by ToggleActionMenu

[David Hurka]

davidhurka updated this revision to Diff 69549.
davidhurka added a comment.


  - Restrict toolbar button popup mode to DelayedPopup and MenuButtonPopup 
using a custom enum
  
  Honestly, I’m not sure whether this is good. We say no one wants to use 
InstantPopup, but as well we could say no one wants DelayedPopup.
  @ngraham?

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21971?vs=69368&id=69549

BRANCH
  create-configurable-toggleactionmenu

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

AFFECTED FILES
  CMakeLists.txt
  ui/pageview.cpp
  ui/toggleactionmenu.cpp
  ui/toggleactionmenu.h
  ui/toolaction.cpp
  ui/toolaction.h

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

D21971: Replace ToolAction by ToggleActionMenu

[David Hurka]

davidhurka added a comment.


  In D21971#560532 , @simgunz wrote:
  
  > In D21971#560495 , @davidhurka 
wrote:
  >
  > > In D21971#559485 , @simgunz 
wrote:
  > >
  > > > To sum it up, if I understand correctly:
  > > >
  > > > - we can remove `InstantPopup` given that we can use `KActionMenu` or 
`KSelectAction` to provide that use case (so no need to raise exceptions)
  > >
  > >
  > > Yes, but how? It can’t simply be forbidden in the constructor argument 
list. Remove it from the body of the constructor, so KActionMenu is set to 
MenuButtonPopup instead?
  >
  >
  > Create a new enum `ToggleActionMenu::ToolButtonPopupMode` with only 
`DelayedPopup` and `MenuButtonPopup`.
  
  
  Sounds reasonable. Can we be sure that no one wants to use `InstantPopup`, 
because `KSelectAction` does that? KSelectAction does not plug in a menu, 
right? I can’t think of such a use case, even the View Mode menu (see D21196 
) wouldn’t fit.
  
  >>> The change to `ToggleActionMenu::defaultAction` is required, otherwise if 
`setDefaultAction` is called on action1 manually and then action2 is checked, 
`defaultAction` returns action1.
  >> 
  >> I can’t follow you here. Yes, when in DefaultLogic mode, and 
setDefaultAction(action1) was called, defaultAction() returns action1. Maybe 
your thinking was focused on checkable actions so far?
  
  I think I understood now what you meant. Your suggested changes to the method 
defaultAction():
  
  In D21971#559080 , @simgunz wrote:
  
  > I suggest to change `ToggleActionMenu::defaultAction` to:
  >
  >   QAction * ToggleActionMenu::defaultAction()
  >   {
  >   if ( ( m_menuLogic & ImplicitDefaultAction ) )
  >   {
  >   QAction * aChecked = checkedAction( menu() );
  >   if ( aChecked )
  >   {
  >   return aChecked;
  >   }
  >   }
  >   return m_defaultAction;
  >   }
  >
  
  
  I thought whether this could give unexpected results, when you trigger an 
action so it becomes unchecked, but there are more checkable options further 
down in the menu. (Imagine Color Mode from D21195 
, with an additional option Preserve 
images.) But then, you wouldn’t use `ImplicitDefaultAction` mode.
  
  But there are more general use cases where this does not work, like the 
Create an additive primitive action menu in FreeCAD. This could be created with 
a ToggleActionMenu in ImplicitDefaultAction mode, by simply adding all actions 
and suggesting/setting a default one.
  F7752159: image.png 
  The last triggered action is not neccessarily checked, especcialy ones which 
are “actions”, not “options” and thus not checkable.
  
  > Try the following code and steps:
  > 
  >   d->aMouseModeMenu = new ToggleActionMenu( QIcon(),QString(), this,
  > 
QToolButton::MenuButtonPopup,
  > 
ToggleActionMenu::ImplicitDefaultAction );
  >   d->aMouseModeMenu->addAction( d->aMouseSelect );
  >   d->aMouseModeMenu->addAction( d->aMouseTextSelect );
  >   d->aMouseModeMenu->addAction( d->aMouseTableSelect );
  >   d->aMouseModeMenu->setDefaultAction( d->aMouseTextSelect );
  >   
  >   ...
  >   
  >   d->aMouseSelect->setChecked( Okular::Settings::mouseMode() == 
Okular::Settings::EnumMouseMode::RectSelect );
  >   d->aMouseTextSelect->setChecked( Okular::Settings::mouseMode() == 
Okular::Settings::EnumMouseMode::TextSelect );
  >   d->aMouseTableSelect->setChecked( Okular::Settings::mouseMode() == 
Okular::Settings::EnumMouseMode::TableSelect );
  > 
  > 
  > 
  > 
  > - Check `Browse Mode`
  > - Restart Okular Results: `Browse Mode` is checked / `Text selection` is 
the default action and unchecked
  > - Check `Text selection`
  > - Restart Okular Results: `Text selection` is the default action and checked
  > - Check `Table Selection`
  > - Restart Okular Results: `Text selection` is the default action and 
unchecked, `Table Selection` is checked but not the default action
  > 
  >   With my change the last case will result in: Results: `Text selection` is 
the default action and checked
  
  Yes, QAction::setChecked() does not emit triggered(), but toggled(). So 
theoreticaly I should connect ToggleActionMenu::setDefaultAction() to 
toggled(), but that’s not emitted by QMenu, so I have to connect to all actions 
in the menu manually. Furthermore, an action could be toggled from outside 
ToggleActionMenu’s menu, e. g. by using the shortcut. It may or may not be 
intended that this updates the toolbar button. When the application itself 
toggles an action, it is usually not intended that the toolbar buttons are 
updated.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabr

[okular] [Bug 413819] highlight annotation not carried over to pdfjs

[Tobias Deiminger]

https://bugs.kde.org/show_bug.cgi?id=413819

--- Comment #15 from Tobias Deiminger  ---
(In reply to Albert Astals Cid from comment #14)
> i'm closing this as upstream since nothing okular does here is wrong, if
> anything at all is wrong it'd be in poppler as Tobias mentioned.
> 
> Tobias i take you're on top of this?

Yep, created https://gitlab.freedesktop.org/poppler/poppler/issues/837 and
https://gitlab.freedesktop.org/poppler/poppler/issues/839 to keep track.

@ghavamikia: If you can reproduce the steps that lead to "okular note" note
even shown in Okular, it would be nice to give more details and snapshots of
the PDF file on its way breaking bad. attachment 123763 is in an inconsistent
state wrt. generation numbers, whatever software caused it should get its own
bug report.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 413745] Add shortcuts for left / right page rotation

[Nate Graham]

https://bugs.kde.org/show_bug.cgi?id=413745

Nate Graham  changed:

   What|Removed |Added

 Depends on|307224  |


Referenced Bugs:

https://bugs.kde.org/show_bug.cgi?id=307224
[Bug 307224] Ability to rotate single pages
-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 307224] Ability to rotate single pages

[Nate Graham]

https://bugs.kde.org/show_bug.cgi?id=307224

Nate Graham  changed:

   What|Removed |Added

 Blocks|413745  |


Referenced Bugs:

https://bugs.kde.org/show_bug.cgi?id=413745
[Bug 413745] Add shortcuts for left / right page rotation
-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 307224] Ability to rotate single pages

[Postix]

https://bugs.kde.org/show_bug.cgi?id=307224

Postix  changed:

   What|Removed |Added

 CC||n...@kde.org

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 413745] Add shortcuts for left / right page rotation

[Postix]

https://bugs.kde.org/show_bug.cgi?id=413745

Postix  changed:

   What|Removed |Added

 Depends on||307224


Referenced Bugs:

https://bugs.kde.org/show_bug.cgi?id=307224
[Bug 307224] Ability to rotate single pages
-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 307224] Ability to rotate single pages

[Postix]

https://bugs.kde.org/show_bug.cgi?id=307224

Postix  changed:

   What|Removed |Added

 Blocks||413745


Referenced Bugs:

https://bugs.kde.org/show_bug.cgi?id=413745
[Bug 413745] Add shortcuts for left / right page rotation
-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 413746] Add option to only rotate specific page(s)

[Postix]

https://bugs.kde.org/show_bug.cgi?id=413746

Postix  changed:

   What|Removed |Added

 Status|CONFIRMED   |RESOLVED
 Resolution|--- |DUPLICATE

--- Comment #2 from Postix  ---
There's 7 years old request for that already. :)

*** This bug has been marked as a duplicate of bug 307224 ***

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 307224] Ability to rotate single pages

[Postix]

https://bugs.kde.org/show_bug.cgi?id=307224

--- Comment #1 from Postix  ---
*** Bug 413746 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 413745] Add shortcuts for left / right page rotation

[Postix]

https://bugs.kde.org/show_bug.cgi?id=413745
Bug 413745 depends on bug 413746, which changed state.

Bug 413746 Summary: Add option to only rotate specific page(s)
https://bugs.kde.org/show_bug.cgi?id=413746

   What|Removed |Added

 Status|CONFIRMED   |RESOLVED
 Resolution|--- |DUPLICATE

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 307224] Ability to rotate single pages

[Postix]

https://bugs.kde.org/show_bug.cgi?id=307224

Postix  changed:

   What|Removed |Added

 CC||pos...@posteo.eu

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 263042] XFA Forms are not supported

[Michael Weghorn]

https://bugs.kde.org/show_bug.cgi?id=263042

--- Comment #19 from Michael Weghorn  ---
(In reply to Albert Astals Cid from comment #18)
> There's non-Free apps on Linux that let you work with XFA files if you
> really really need to, use your favourite search engine to find them.

And PDFium, which is used e.g. by Chromium, has some support for XFA. When I
looked at this a while ago (probably about two years ago) and tested a few
files, that turned out not to be far from ideal, but that *might* have changed
in the meantime. (Back then, XFA support was not enabled by default, but there
was a compile option).

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 413745] Add shortcuts for left / right page rotation

[Albert Astals Cid]

https://bugs.kde.org/show_bug.cgi?id=413745

--- Comment #12 from Albert Astals Cid  ---
I don't think those are duplicates no, one is about single page rotation and
the other about shortcuts for the rotation action.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 413966] Fails to open PDF document

[Albert Astals Cid]

https://bugs.kde.org/show_bug.cgi?id=413966

--- Comment #3 from Albert Astals Cid  ---
Personally not interested, i have lots of things to fix that can be done on
public documents.

-- 
You are receiving this mail because:
You are the assignee for the bug.

D21971: Replace ToolAction by ToggleActionMenu

[Simone Gaiarin]

simgunz added a comment.


  In D21971#560495 , @davidhurka 
wrote:
  
  > In D21971#559485 , @simgunz 
wrote:
  >
  > > To sum it up, if I understand correctly:
  > >
  > > - we can remove `InstantPopup` given that we can use `KActionMenu` or 
`KSelectAction` to provide that use case (so no need to raise exceptions)
  >
  >
  > Yes, but how? It can’t simply be forbidden in the constructor argument 
list. Remove it from the body of the constructor, so KActionMenu is set to 
MenuButtonPopup instead?
  
  
  Create a new enum `ToggleActionMenu::ToolButtonPopupMode` with only 
`DelayedPopup` and `MenuButtonPopup`.
  
  >> - you agree in removing `DefaultLogic`
  > 
  > No, it is needed when other actions than the exclusive group are added to 
the menu.
  >  Example is D21195 , where an action to 
configure the color modes is added to the Color Mode menu. When you click that 
action in ImplicitDefaultAction mode, it would become the default action, and 
the toolbar tells you that you are in color mode “Configure...”.
  >  Other option is to allow only checkable actions to become the default 
action. But then, you can’t add real “actions” (opposed to “tools”). Imagine 
the user has some construction plan, and the application has several actions of 
kind “Mark for ”, and the user wants to apply one of these action to 
several items. Then it could be useful to collect all “Mark for ” actions 
in a ToggleActionMenu ImplicitDefaultAction mode, while the actions can not be 
checked.
  
  ok
  
  >> The change to `ToggleActionMenu::defaultAction` is required, otherwise if 
`setDefaultAction` is called on action1 manually and then action2 is checked, 
`defaultAction` returns action1.
  > 
  > I can’t follow you here. Yes, when in DefaultLogic mode, and 
setDefaultAction(action1) was called, defaultAction() returns action1. Maybe 
your thinking was focused on checkable actions so far?
  
  Try the following code and steps:
  
d->aMouseModeMenu = new ToggleActionMenu( QIcon(),QString(), this,
  QToolButton::MenuButtonPopup,
  
ToggleActionMenu::ImplicitDefaultAction );
d->aMouseModeMenu->addAction( d->aMouseSelect );
d->aMouseModeMenu->addAction( d->aMouseTextSelect );
d->aMouseModeMenu->addAction( d->aMouseTableSelect );
d->aMouseModeMenu->setDefaultAction( d->aMouseTextSelect );

...

d->aMouseSelect->setChecked( Okular::Settings::mouseMode() == 
Okular::Settings::EnumMouseMode::RectSelect );
d->aMouseTextSelect->setChecked( Okular::Settings::mouseMode() == 
Okular::Settings::EnumMouseMode::TextSelect );
d->aMouseTableSelect->setChecked( Okular::Settings::mouseMode() == 
Okular::Settings::EnumMouseMode::TableSelect );
  
  
  
  - Check `Browse Mode`
  - Restart Okular
  
  Results: `Browse Mode` is checked / `Text selection` is the default action 
and unchecked
  
  - Check `Text selection`
  - Restart Okular
  
  Results: `Text selection` is the default action and checked
  
  - Check `Table Selection`
  - Restart Okular
  
  Results: `Text selection` is the default action and unchecked / `Table 
Selection` is checked but not the default action
  
  With my change the last case will result in:
  Results: `Text selection` is the default action and checked
  
  >> [...]
  >>  Thinking forward, `ToggleActionMenu` could become just a 'mode' of 
`KActionMenu`. Right?
  > 
  > Yes. But I would like Bug 413827 
 to be resolved (so PopupMode 
enumerator is used).
  
  ok

REPOSITORY
  R223 Okular

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

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