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

Enable editor when double-clicking on stamp annotation #19320

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

nicolo-ribaudo
Copy link
Contributor

In Firefox, double-clicking on a stamp annotation triggers text selection (selecting the last text element in the dom before the annotation): this triggers the logic to make annotations not interfere with text selection, which in turns prevents the double click from triggering the annotation editor.

This commit fixes the problem by making annotations non-selectable, so that clicking on them does not trigger text selection. Freetext annotations were already non-selectable, so this commit doesn't change that. However, we need to explicitly mark text in popups as selectable.

Fixes #19315

Note that only annotations that are included in the PDF are editable by double-clicking on them. All user-created annotations are not, because they are only in the annotationEditorLayer what has pointer-events: none.

@Snuffleupagus
Copy link
Collaborator

Unfortunately the new test doesn't seem to work on the bots, since both logs contain:

3) Stamp Editor Switch to edit mode by double clicking on an existing stamp annotation must switch to edit mode
  Message:
�[31m    Expected 'annotationEditorLayer disabled' to contain 'stampEditing'.�[0m
  Stack:
        at <Jasmine>
        at file:///home/ubuntu/pdfjs/botio-files-pdfjs/private/20f512b4c185955/test/integration/stamp_editor_spec.mjs:1843:43
        at async Promise.all (index 1)
        at async UserContext.<anonymous> (file:///home/ubuntu/pdfjs/botio-files-pdfjs/private/20f512b4c185955/test/integration/stamp_editor_spec.mjs:1833:7)

@nicolo-ribaudo
Copy link
Contributor Author

Taking at the look at the failure, since it seems likely to be caused by my changes

@mozilla mozilla deleted a comment from moz-tools-bot Jan 14, 2025
@mozilla mozilla deleted a comment from moz-tools-bot Jan 14, 2025
@mozilla mozilla deleted a comment from moz-tools-bot Jan 14, 2025
@mozilla mozilla deleted a comment from moz-tools-bot Jan 14, 2025

Verified

This commit was signed with the committer’s verified signature.
nicolo-ribaudo Nicolò Ribaudo
In Firefox, double-clicking on a stamp annotation triggers text
selection (selecting the last text element in the dom before the
annotation): this triggers the logic to make annotations not interfere
with text selection, which in turns prevents the double click from
triggering the annotation editor.

This commit fixes the problem by making annotations non-selectable, so
that clicking on them does not trigger text selection. Freetext
annotations were already non-selectable, so this commit doesn't change
that. However, we need to explicitly mark text in popups as selectable.
@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/1ec021137f9e021/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/753b7df4a6ab1dd/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/753b7df4a6ab1dd/output.txt

Total script time: 10.50 mins

  • Integration Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/1ec021137f9e021/output.txt

Total script time: 25.87 mins

  • Integration Tests: FAILED

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

r=me, thank you.

@Snuffleupagus Snuffleupagus removed the request for review from calixteman January 14, 2025 11:02
@Snuffleupagus Snuffleupagus merged commit 016de74 into mozilla:master Jan 14, 2025
7 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the stamp-double-click branch January 14, 2025 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Cannot edit a stamp annotation in double-clicking on it
4 participants