D10865: [1/5] Access readOnly state of FormWidgets dynamically

[Albert Astals Cid]

aacid added inline comments.

INLINE COMMENTS

> aheinecke wrote in formwidgets.cpp:310
> As I understand it there is a case where Forms are bisible, but disabled.
> This is if in "PageView::notifySetup" the check for:
> 
>   const bool allowfillforms = d->document->isAllowed( Okular::AllowFillForms 
> );
> 
> Returns false.
> Then Okular will show all FormWidgets which are not readOnly disabled. I'm 
> not sure if that makes much sense but as there is the extra code with 
> "setCanBeFilled" I thought I should better leave that behavior because 
> someone intended it that way at some point ;-)

sure, if allowfillforms is false, we will call setCanBeFilled with false and it 
will be setEnabled to false.

What I am asking is why do we need to call isReadOnly here. As far as i 
understand if the field is readonly, it won't be shown, so the enabled status 
of it doesn't matter, no?

REPOSITORY
  R223 Okular

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

To: aheinecke, #okular
Cc: aacid, michaelweghorn, ngraham

D10932: [Okular] Option to reset forms

[Albert Astals Cid]

aacid requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R223 Okular

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

To: ahmadosama, #okular, aacid
Cc: ngraham, aacid, #okular, michaelweghorn

D10792: Raise annotation window when clicking on annotation

[Albert Astals Cid]

aacid added a comment.


  ping?

REPOSITORY
  R223 Okular

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

To: simgunz, #okular, aacid
Cc: ngraham, #okular, michaelweghorn, aacid

D11051: Remembering side navigation panel state

[Albert Astals Cid]

aacid requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R223 Okular

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

To: dileepsankhla, #okular, aacid
Cc: aacid, sander, #okular, michaelweghorn, ngraham

D10797: Add asterisk when an annotation is associated to non-empty popup

[Albert Astals Cid]

aacid requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R223 Okular

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

To: simgunz, #okular, aacid
Cc: aacid, ngraham, michaelweghorn

D10504: Bug 288042 - Option to reset forms

[Albert Astals Cid]

aacid added a comment.


  So will you cancel this one in favor of the other one?

REPOSITORY
  R223 Okular

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

To: ahmadosama, aacid
Cc: aacid, ngraham, #okular, michaelweghorn

[okular] [Bug 391352] Trim margins breaks mouse wheel zoom out

[Albert Astals Cid]

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

Albert Astals Cid  changed:

   What|Removed |Added

 Status|NEEDSINFO   |CONFIRMED
Summary|Okular does not zoom out|Trim margins breaks mouse
   |with mouser wheel.  |wheel zoom out
 Ever confirmed|0   |1
 Resolution|WAITINGFORINFO  |---

--- Comment #12 from Albert Astals Cid  ---
Maybe 342003 but not sure. Anyhow we can just use this one.

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

D11051: Remembering side navigation panel state

[Albert Astals Cid]

aacid added a comment.


  In D11051#222803 , @dileepsankhla 
wrote:
  
  > In D11051#05 , @aacid wrote:
  >
  > > Also i'm not convinced it's a good idea to save the settings in 
sidebar.cpp but read them in part.cpp, seems a bit asymmetrical, can you 
explain your reasoning for it?
  >
  >
  > Initially, I thought of reading and saving the settings in `part.cpp` but I 
can't access `m_sidebar` as to save its last state inside the destructor 
`Part::~Part` and it gives me segfault. Hence I am saving them in 
`Sidebar::~Sidebar`.
  
  
  I just added 
  qDebug() << m_sidebar->isCollapsed();
  to Part destructor and it's all fine, i don't see why you would get a 
segfault, and if you did, you should investigate why, if you thought that Part 
destructor was a better place instead of just saying "meh i've no idea why it 
happens, let's put it in a worse place"

REPOSITORY
  R223 Okular

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

To: dileepsankhla, #okular
Cc: aacid, sander, #okular, michaelweghorn, ngraham

D10932: [Okular] Option to reset forms

[Albert Astals Cid]

aacid added a comment.


  Have you thought how this should interact with the undo/redo history stack?

INLINE COMMENTS

> resetformstest.cpp:182
> +{
> +verifyTextForm( m_textLineForm );
> +}

what does this test do?

> part.cpp:3359
>  m_formsMessage->addAction( m_pageView->toggleFormsAction() );
> -
> +m_formsMessage->addAction( m_pageView->resetFormsAction() );
>  // ensure history actions are in the correct state

Why did you add this?

> pageview.cpp:731
> +d->aResetForms->setEnabled( false );
> +resetFormWidgets( false );
> +

Why are you calling this?

> pageview.cpp:4333
> +if(d->aResetForms)
> +d->aResetForms->setText( i18n( "Reset Forms" ) );
> +if( d->aResetForms && on)

why do you set the text every single time this is called  instead of on 
construction time?

> pageview.cpp:4433
> +{
> +document->editFormButtons( pageNumber, formButtons, newButtonStates);
> +}

What's the point of all this one single line functions?

REPOSITORY
  R223 Okular

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

To: ahmadosama, #okular
Cc: ngraham, aacid, #okular, michaelweghorn

[okular] [Bug 288042] Option to reset forms (PDF)

[Ahmad Osama]

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

--- Comment #9 from Ahmad Osama  ---
I created an auto-test for this option that is working fine on the created test
cases, the new test along with the implementation for this option are in this
patch:

https://phabricator.kde.org/D10932

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

D10932: [Okular] Option to reset forms

[Ahmad Osama]

ahmadosama updated this revision to Diff 29396.
ahmadosama edited the test plan for this revision.
ahmadosama added a comment.


  I added an auto-test for this option, the test is working fine on all the 
created test cases.

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10932?vs=29292=29396

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/resetformstest.cpp
  part.cpp
  part.h
  part.rc
  ui/pageview.cpp
  ui/pageview.h

To: ahmadosama, #okular
Cc: ngraham, aacid, #okular, michaelweghorn

D10932: [Okular] Option to reset forms

[Ahmad Osama]

ahmadosama retitled this revision from "Bug 288042 - Option to reset forms 
(Implemented using FormFields)" to "[Okular] Option to reset forms".
ahmadosama edited the summary of this revision.
ahmadosama edited the test plan for this revision.

REPOSITORY
  R223 Okular

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

To: ahmadosama, #okular
Cc: ngraham, aacid, #okular, michaelweghorn

KDE CI: Applications okular kf5-qt5 FreeBSDQt5.9 - Build # 49 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Applications%20okular%20kf5-qt5%20FreeBSDQt5.9/49/
 Project:
Applications okular kf5-qt5 FreeBSDQt5.9
 Date of build:
Tue, 13 Mar 2018 08:29:27 +
 Build duration:
1 hr 12 min and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 14 test(s), Skipped: 0 test(s), Total: 16 test(s)Failed: TestSuite.mainshelltestFailed: TestSuite.parttest