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.

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

Ensure that automatic printing still works when the viewer and/or its pages are hidden (bug 1618621, bug 1618955) #11646

Merged
merged 2 commits into from
Mar 25, 2020
Merged

Ensure that automatic printing still works when the viewer and/or its pages are hidden (bug 1618621, bug 1618955) #11646

merged 2 commits into from
Mar 25, 2020

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Mar 1, 2020 •
edited
Loading

Please note that this patch, on its own, won't magically fix all of these printing bugs without bug 1618553 also being fixed.
(However I don't foresee that being too difficult, famous last words :-), but it will as suggested require a platform API that we can notify when the viewer is ready.)

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1618621
Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1618955
Fixes #8208

/cc @brendandahl

@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch from f709ff4 to 948744e Compare March 1, 2020 14:02
@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch 9 times, most recently from 5249f35 to f6efc50 Compare March 2, 2020 10:24
@pdfjsbot
Copy link

pdfjsbot commented Mar 2, 2020

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.67.70.0:8877/99f12f6019b3f2b/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 2, 2020

From: Bot.io (Linux m4)

Success

Full output at http://54.67.70.0:8877/99f12f6019b3f2b/output.txt

Total script time: 2.40 mins

Published

@Snuffleupagus
Copy link
Collaborator Author

While we still need to sort out the "load" event, in order to fix all of these issues completely, even as-is this PR is a considerable improvement in my testing.

Having tried all of the test-cases from the bugs/issues linked in #11646 (comment) in a Firefox build with this patch applied, they all work now if the user triggers printing manually since that gives the viewer/document enough time to load.
(Only the immediate automatic printing on load in an <iframe> is still broken, pending the "load" event fix.)

@brendandahl Can we land this PR now, since it's ready, rather than waiting for the "load" event fix?

@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch from f6efc50 to ff12b68 Compare March 7, 2020 14:13
@pdfjsbot
Copy link

pdfjsbot commented Mar 7, 2020

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.67.70.0:8877/323703b7628bf56/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 7, 2020

From: Bot.io (Linux m4)

Success

Full output at http://54.67.70.0:8877/323703b7628bf56/output.txt

Total script time: 2.47 mins

Published

@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch from ff12b68 to d76a9e3 Compare March 9, 2020 22:29
@pdfjsbot
Copy link

pdfjsbot commented Mar 9, 2020

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.67.70.0:8877/e91f451adb01d9b/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 9, 2020

From: Bot.io (Linux m4)

Success

Full output at http://54.67.70.0:8877/e91f451adb01d9b/output.txt

Total script time: 2.52 mins

Published

@Snuffleupagus
Copy link
Collaborator Author

@brendandahl Gentle review ping; as outlined in #11646 (comment) this patch improves the current situation enough to warrant landing as soon as possible in my opinion :-)

@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch from d76a9e3 to e76abc7 Compare March 14, 2020 10:12
@Snuffleupagus
Copy link
Collaborator Author

Rebasing onto master to ensure that the patch still applies cleanly; no code changes were made.

@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch 2 times, most recently from fcad067 to d0617cd Compare March 17, 2020 21:53
brendandahl
brendandahl suggested changes Mar 18, 2020
View reviewed changes
@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch from d0617cd to 174b4a3 Compare March 24, 2020 12:56
@Snuffleupagus
Copy link
Collaborator Author

/botio unittest

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.67.70.0:8877/ff778660e30c280/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

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

Live output at: http://54.215.176.217:8877/84e216dfa388322/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Success

Full output at http://54.67.70.0:8877/ff778660e30c280/output.txt

Total script time: 2.70 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)

Success

Full output at http://54.215.176.217:8877/84e216dfa388322/output.txt

Total script time: 4.15 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.67.70.0:8877/bb30d99dbec6bf1/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Success

Full output at http://54.67.70.0:8877/bb30d99dbec6bf1/output.txt

Total script time: 2.51 mins

Published

@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch from 542396d to 6dcdbea Compare March 24, 2020 15:24
@Snuffleupagus
Ensure that automatic printing still works when the viewer and/or its…
c5b0b5c 
… pages are hidden (bug 1618621, bug 1618955)

Please note that this patch, on its own, won't magically fix all of these printing bugs without [bug 1618553](https://bugzilla.mozilla.org/show_bug.cgi?id=1618553) also being fixed.
(However I don't foresee that being too difficult, famous last words :-), but it will as suggested require a platform API that we can notify when the viewer is ready.)

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1618621
Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1618955
Fixes 8208
@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch from 6dcdbea to 6286006 Compare March 24, 2020 15:26
@Snuffleupagus
Rename BaseViewer._setDocumentViewerElement to `BaseViewer._viewerE…
a24ad28 
…lement`

It was pointed out the the old name felt confusing, so let's just rename the getter since it's an internal property anyway.
@Snuffleupagus Snuffleupagus force-pushed the _setDocumentAllowFetchPages branch from 6286006 to a24ad28 Compare March 24, 2020 15:55
@timvandermeij timvandermeij merged commit 8745286 into mozilla:master Mar 25, 2020
@timvandermeij
Copy link
Contributor

Thanks!

@Snuffleupagus Snuffleupagus deleted the _setDocumentAllowFetchPages branch March 26, 2020 09:20
@snyk-bot snyk-bot mentioned this pull request Mar 7, 2022
Open
@yoonpsu yoonpsu mentioned this pull request Apr 20, 2022
Open
@yoonpsu yoonpsu mentioned this pull request Oct 25, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Nov 5, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brendandahl brendandahl brendandahl approved these changes

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

Successfully merging this pull request may close these issues.

FireFox - "offsetParent is not set -- cannot scroll viewer.js:125:3"
4 participants
@Snuffleupagus @pdfjsbot @timvandermeij @brendandahl