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

[Andre Heinecke]

aheinecke added a comment.


  In D10865#217744 <https://phabricator.kde.org/D10865#217744>, @aacid wrote:
  
  > BTW next time  please use arc so phabricator shows the context of the diff.
  
  
  Apologies, I'll try. While I like phabricator I'm not very skilled with arcanist, yet. :-}

INLINE COMMENTS

> aacid wrote in formwidgets.cpp:304
> i would really prefer this to be moved to whoever calls setVisiblity, it's kind of weird for setVisiblity to sometimes not do what you ask it to do.

Yes, Indeed. I'll update the Differential accordingly.

> aacid wrote in formwidgets.cpp:310
> Do we need this at all? the widget won't ge visible when it's readonly, so what do we care if it's enabled or not?

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 ;-)

> aacid wrote in pageview.cpp:1002
> If we remove the use of visiblity as i commented already we can get this back to how it was?

No, setCanBeFilled accesses isReadOnly. This crashes if the formWidgets are not yet updated with the new fields.

I also think that it is better to only modify the field here and not earlier to avoid working with formWidgets that have dangling pointers to deleted fields in them.

REPOSITORY
  R223 Okular

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

To: aheinecke, #okular
Cc: aacid, michaelweghorn, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20180305/bf794c3d/attachment.html>