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

Make sure that the document is rendered on zooming and rotation for PDFViewer instances using the defaultRenderingQueue #6157

Merged
merged 1 commit into from
Jul 16, 2015
Merged

Make sure that the document is rendered on zooming and rotation for PDFViewer instances using the defaultRenderingQueue #6157

merged 1 commit into from
Jul 16, 2015

Conversation

Snuffleupagus
Copy link
Collaborator

In viewer.js we have code that ensures that the document is re-rendered on zooming and rotation. However, for components based viewers this might not work correctly, since there's currently no code which handles that.

Note that there is a good chance that this "just works" in many components viewers already, thanks to the watchScroll function.
The explanation is that zooming or rotation, most of the time, causes the viewer to change its (scrollable) size, thus triggering PDFViewer_update through PDFViewer_scrollUpdate.

However, in general there's no guarantee that this will actually work (since zooming and rotation doesn't necessarily change the size of the viewer for all documents), and requiring every viewer components implementer to provide methods for this doesn't seem like a great idea.

…PDFViewer` instances using the `defaultRenderingQueue`

In `viewer.js` we have code that ensures that the document is re-rendered on zooming and rotation. However, for `components` based viewers this might not work correctly, since there's currently no code which handles that.

Note that there is a good chance that this "just works" in many `components` viewers already, thanks to the `watchScroll` function.
The explanation is that zooming or rotation, most of the time, causes the viewer to change its (scrollable) size, thus triggering `PDFViewer_update` through `PDFViewer_scrollUpdate`.

However, in general there's no guarantee that this will actually work (since zooming and rotation doesn't necessarily change the size of the viewer for all documents), and requiring every viewer `components` implementer to provide methods for this doesn't seem like a great idea.
@Snuffleupagus
Copy link
Collaborator Author

@timvandermeij Would you be up for reviewing this patch?
To test this, compare the patch with the current master (remember to do node make generic components after applying the patch), by visiting the simpleviewer example locally. Then open the console (Ctrl+Shift+K in Firefox), and execute e.g. the following commands:

  • pdfViewer.currentScaleValue = 1;
  • pdfViewer.pagesRotation = 90;

@timvandermeij timvandermeij self-assigned this Jul 16, 2015
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

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

Live output at: http://107.21.233.14:8877/32f9b6f16511bd4/output.txt

timvandermeij added a commit that referenced this pull request Jul 16, 2015
…cale-and-pagesRotation

Make sure that the document is rendered on zooming and rotation for `PDFViewer` instances using the `defaultRenderingQueue`
@timvandermeij timvandermeij merged commit b35cbaa into mozilla:master Jul 16, 2015
@timvandermeij
Copy link
Contributor

Looks good, thank you for the patch and for providing the clear testing steps!

@Snuffleupagus Snuffleupagus deleted the components-update-on-setScale-and-pagesRotation branch July 17, 2015 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants