Skip to content

[api-minor] Render pushbuttons on their own canvas (bug 1737260) #14247

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 16, 2021
Merged

[api-minor] Render pushbuttons on their own canvas (bug 1737260) #14247

merged 1 commit into from
Nov 16, 2021

Conversation

calixteman
Copy link
Contributor

@calixteman calixteman commented Nov 6, 2021 •
edited
Loading

  • First step to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1737260;
  • several interactive pdfs use the possibility to hide/show buttons to show different icons;
  • render pushbuttons on their own canvas and then insert it the annotation_layer;
  • update test/driver.js in order to convert canvases for pushbuttons into images.

@calixteman calixteman force-pushed the button branch 2 times, most recently from f51badb to a092191 Compare November 6, 2021 21:27
@calixteman calixteman changed the title Render pushbuttons on their own canvas [api-minor] Render pushbuttons on their own canvas Nov 6, 2021
Snuffleupagus
Snuffleupagus reviewed Nov 6, 2021
View reviewed changes
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment •
edited
Loading

Choose a reason for hiding this comment

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

A couple of initial comments, based on a quick look at this patch.

@calixteman
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Nov 8, 2021

From: Bot.io (Windows)

Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/b91c780f3697f40/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Nov 8, 2021

From: Bot.io (Linux m4)

Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/35402b905beec72/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Nov 8, 2021

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/35402b905beec72/output.txt

Total script time: 23.52 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 12
  different first/second rendering: 2

Image differences available at: http://54.241.84.105:8877/35402b905beec72/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

pdfjsbot commented Nov 8, 2021

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/b91c780f3697f40/output.txt

Total script time: 41.71 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 15
  different first/second rendering: 1

Image differences available at: http://54.193.163.58:8877/b91c780f3697f40/reftest-analyzer.html#web=eq.log

@brendandahl
Copy link
Contributor

The button in issue12716.pdf is no longer scaled correctly with this patch:

image

Snuffleupagus
Snuffleupagus reviewed Nov 8, 2021
View reviewed changes
Snuffleupagus
Snuffleupagus reviewed Nov 8, 2021
View reviewed changes
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Another thing that I'm guessing doesn't currently work is the useOnlyCssZoom mode, when set either explicitly in the appOptions or implicitly by increasing the zoom level (a lot) in the viewer.

Most likely you'll need to modify the width/height of the annotationCanvases in that case, perhaps similar to how the main page-canvas is handled when CSS-zooming is being used.

Snuffleupagus
Snuffleupagus reviewed Nov 9, 2021
View reviewed changes
@calixteman calixteman force-pushed the button branch 2 times, most recently from 632f718 to 8a8dcfd Compare November 11, 2021 19:38
@calixteman
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/3079be7f4e522be/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/6ae9667fd4c42c3/output.txt

Snuffleupagus
Snuffleupagus reviewed Nov 11, 2021
View reviewed changes
@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/3079be7f4e522be/output.txt

Total script time: 23.65 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 12

Image differences available at: http://54.241.84.105:8877/3079be7f4e522be/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/6ae9667fd4c42c3/output.txt

Total script time: 42.01 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 15
  different first/second rendering: 1

Image differences available at: http://54.193.163.58:8877/6ae9667fd4c42c3/reftest-analyzer.html#web=eq.log

@calixteman
[api-minor] Render pushbuttons on their own canvas (bug 1737260)
33ea817 
 - First step to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1737260;
 - several interactive pdfs use the possibility to hide/show buttons to show different icons;
 - render pushbuttons on their own canvas and then insert it the annotation_layer;
 - update test/driver.js in order to convert canvases for pushbuttons into images.
@calixteman calixteman changed the title [api-minor] Render pushbuttons on their own canvas [api-minor] Render pushbuttons on their own canvas (bug 1737260) Nov 12, 2021
@brendandahl
Copy link
Contributor

/botio makeref

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Received

Command cmd_makeref from @brendandahl received. Current queue size: 0

Live output at: http://54.241.84.105:8877/96a8b3b61268066/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

Command cmd_makeref from @brendandahl received. Current queue size: 0

Live output at: http://54.193.163.58:8877/b7e9086d8e49047/output.txt

@brendandahl brendandahl merged commit 3209c01 into mozilla:master Nov 16, 2021
@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/96a8b3b61268066/output.txt

Total script time: 21.49 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/b7e9086d8e49047/output.txt

Total script time: 39.11 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@timvandermeij timvandermeij removed their request for review November 17, 2021 18:38
@quaoaris
Copy link
Contributor

quaoaris commented Jan 8, 2022

After this merge:
commit 3209c01
Merge: e4f97a2 33ea817
Author: Brendan Dahl [email protected]
Date: Tue Nov 16 08:10:40 2021 -0800

The buttons on testcase in https://bugzilla.mozilla.org/show_bug.cgi?id=1737260 are not visible anymore. Tested on Win10 with FF.

calixteman added a commit to calixteman/pdf.js that referenced this pull request Feb 19, 2022
@calixteman
[Annotations] Show buttons even if they've no actions
3802470 
- it's a regression from PR mozilla#14247:
 - before the PR, the button was rendered on the canvas whatever its status was;
 - after the PR, the button image has been moved in an other canvas so when the button is not renderable
   (because it has no actions) then the image is not added the HTML element.
- the buttons in the pdf in bug 1737260 or in the pdf in mozilla#14308 were not visible
- make the button always renderable but don't add the link element if it's useless.
calixteman added a commit to calixteman/pdf.js that referenced this pull request Feb 19, 2022
@calixteman
[Annotations] Show buttons even if they've no actions
571fe0e 
- it's a regression from PR mozilla#14247:
 - before the PR, the button was rendered on the canvas whatever its status was;
 - after the PR, the button image has been moved in an other canvas so when the button is not renderable
   (because it has no actions) then the image is not added the HTML element.
- the buttons in the pdf in bug 1737260 or in the pdf in mozilla#14308 were not visible
- make the button always renderable but don't add the link element if it's useless.
@calixteman calixteman mentioned this pull request Feb 19, 2022
Merged
calixteman added a commit to calixteman/pdf.js that referenced this pull request Feb 20, 2022
@calixteman
[Annotations] Show buttons even if they've no actions
90d13df 
- it's a regression from PR mozilla#14247:
 - before the PR, the button was rendered on the canvas whatever its status was;
 - after the PR, the button image has been moved in an other canvas so when the button is not renderable
   (because it has no actions) then the image is not added the HTML element.
- the buttons in the pdf in bug 1737260 or in the pdf in mozilla#14308 were not visible
- make the button always renderable but don't add the link element if it's useless.
calixteman added a commit to calixteman/pdf.js that referenced this pull request Mar 12, 2022
@calixteman
[Annotations] Show buttons even if they've no actions
f4f2c0f 
- it's a regression from PR mozilla#14247:
 - before the PR, the button was rendered on the canvas whatever its status was;
 - after the PR, the button image has been moved in an other canvas so when the button is not renderable
   (because it has no actions) then the image is not added the HTML element.
- the buttons in the pdf in bug 1737260 or in the pdf in mozilla#14308 were not visible
- make the button always renderable but don't add the link element if it's useless.
@tobidelbruck tobidelbruck mentioned this pull request Mar 30, 2022
Open
calixteman added a commit to calixteman/pdf.js that referenced this pull request May 2, 2022
@calixteman
[Annotations] Show buttons even if they've no actions
d2769eb 
- it's a regression from PR mozilla#14247:
 - before the PR, the button was rendered on the canvas whatever its status was;
 - after the PR, the button image has been moved in an other canvas so when the button is not renderable
   (because it has no actions) then the image is not added the HTML element.
- the buttons in the pdf in bug 1737260 or in the pdf in mozilla#14308 were not visible
- make the button always renderable but don't add the link element if it's useless.
@Snuffleupagus Snuffleupagus mentioned this pull request May 8, 2022
Merged
calixteman added a commit to calixteman/pdf.js that referenced this pull request May 28, 2022
@calixteman
[Annotations] Show buttons even if they've no actions
66b513f 
- it's a regression from PR mozilla#14247:
 - before the PR, the button was rendered on the canvas whatever its status was;
 - after the PR, the button image has been moved in an other canvas so when the button is not renderable
   (because it has no actions) then the image is not added the HTML element.
- the buttons in the pdf in bug 1737260 or in the pdf in mozilla#14308 were not visible
- make the button always renderable but don't add the link element if it's useless.
bh213 pushed a commit to bh213/pdf.js that referenced this pull request Jun 3, 2022
@calixteman @bh213
[Annotations] Show buttons even if they've no actions
2c0e74c 
- it's a regression from PR mozilla#14247:
 - before the PR, the button was rendered on the canvas whatever its status was;
 - after the PR, the button image has been moved in an other canvas so when the button is not renderable
   (because it has no actions) then the image is not added the HTML element.
- the buttons in the pdf in bug 1737260 or in the pdf in mozilla#14308 were not visible
- make the button always renderable but don't add the link element if it's useless.
rousek pushed a commit to signosoft/pdf.js that referenced this pull request Aug 10, 2022
@calixteman @rousek
[Annotations] Show buttons even if they've no actions
e48b691 
- it's a regression from PR mozilla#14247:
 - before the PR, the button was rendered on the canvas whatever its status was;
 - after the PR, the button image has been moved in an other canvas so when the button is not renderable
   (because it has no actions) then the image is not added the HTML element.
- the buttons in the pdf in bug 1737260 or in the pdf in mozilla#14308 were not visible
- make the button always renderable but don't add the link element if it's useless.
@Snuffleupagus Snuffleupagus mentioned this pull request Oct 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brendandahl brendandahl brendandahl left review comments

@Snuffleupagus Snuffleupagus Snuffleupagus left review comments

Assignees
No one assigned
Labels
annotations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@calixteman @pdfjsbot @brendandahl @quaoaris @Snuffleupagus