Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cool#9992 doc sign: add UI to sign PDF files #10431

Merged
merged 2 commits into from
Nov 7, 2024
Merged

Conversation

vmiklos
Copy link
Contributor

@vmiklos vmiklos commented Nov 7, 2024

Open a PDF file, it is loaded into a read-only Draw view, so the File
menu has no signing menu item to sign the document.

There are some menu items, but all of those are actions, and one of
those are inserting a comment: so probably the intent is to avoid not
necessary modifications to the PDF file (like delete the only bitmap on
the draw page), than to actually avoid modifying the document.

Similar to how we have an allowlist for actions, add an allowlist for
UNO commands which are OK to dispatch in read-only mode: just have the
signature command there for now.

In the future, we may want to revisit this to also have a more strict
read-only mode without commenting/signing, but treating signing similar
to commenting is probably good enough for now.

Signed-off-by: Miklos Vajna vmiklos@collabora.com
Change-Id: Ib3b396f110cd2d99dbb5d4704f5733bb135f891e

Open a PDF file, it is loaded into a read-only Draw view, so the File
menu has no signing menu item to sign the document.

There are some menu items, but all of those are actions, and one of
those are inserting a comment: so probably the intent is to avoid not
necessary modifications to the PDF file (like delete the only bitmap on
the draw page), than to actually avoid modifying the document.

Similar to how we have an allowlist for actions, add an allowlist for
UNO commands which are OK to dispatch in read-only mode: just have the
signature command there for now.

In the future, we may want to revisit this to also have a more strict
read-only mode without commenting/signing, but treating signing similar
to commenting is probably good enough for now.

Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
Change-Id: Ib3b396f110cd2d99dbb5d4704f5733bb135f891e
@vmiklos vmiklos mentioned this pull request Nov 7, 2024
@vmiklos
Copy link
Contributor Author

vmiklos commented Nov 7, 2024

https://cpci.cbg.collabora.co.uk:8080/job/github_online_master_debug_vs_co-24.04_cypress_desktop/2287/console fails in:

writer/file_properties_spec.js

but

make -C cypress_test check-desktop spec=writer/file_properties_spec.js

passes for me locally (failed once, but then passed..), retry.

It seems that the primary problem is that the viewport is scroll, so the
tab dialog can be scrolled and cypress scrolls to the middle by default.

Unfortunately scrolling to the buttons that would do tab switching
doesn't seem to help: as soon as the button is available, cypress clicks
on it, and somehow that's too early.

Drop 3 no more needed waits and add back 2 elsewhere, now this test
passes for me locally several times in a row.

Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
Change-Id: I90901750c812ffec1914ab09877d71c0fabf7996
@vmiklos
Copy link
Contributor Author

vmiklos commented Nov 7, 2024

https://cpci.cbg.collabora.co.uk:8080/job/github_online_master_debug_vs_co-24.04_cypress_desktop/2289/console failed again in:

writer/file_properties_spec.js

let me try to make that more stable, then.

@vmiklos vmiklos requested a review from caolanm November 7, 2024 10:49
@vmiklos
Copy link
Contributor Author

vmiklos commented Nov 7, 2024

@caolanm could you please review this? Thanks.

With this, interactive PDF signing works for me in COOL. The 2nd commit tries to improve the stability of that cypress test -- I don't see any change to the stability with or without the 1st commit.

@@ -13,25 +13,21 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'File Property Tests', func

it('Add File Description.', function() {
writerHelper.openFileProperties();
// Too early click on the 'Description' button would have no effect.
cy.wait(500);
cy.cGet('#tabcontrol-2').click();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine its not as straightforward as the button is disabled until scrolled into place and async enabled just after cypress clicks on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, possible, I'll keep that in mind next time we run into something like this. I know click() loudly fails when you try to click on something not visible, but possibly it's different for the "visible, but disabled" case.

@caolanm caolanm merged commit 3e5ad8f into master Nov 7, 2024
13 checks passed
@caolanm caolanm deleted the private/vmiklos/master branch November 7, 2024 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants