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

Fix scroll to zoom jump between 25% and 1000% #6014

Merged
merged 1 commit into from
May 14, 2015
Merged

Fix scroll to zoom jump between 25% and 1000% #6014

merged 1 commit into from
May 14, 2015

Conversation

xlc
Copy link
Contributor

@xlc xlc commented May 14, 2015

Fixes #6013

@Snuffleupagus
Copy link
Collaborator

I'm sorry, but this patch seem to "break" normal scroll wheels, since the loop in zoomIn/zoomOut now runs one more time than it should. I'm assuming that the actual issue this patch tries to address is when zoomIn/zoomOut is called with the ticks argument set to 0.

Snuffleupagus
Snuffleupagus reviewed May 14, 2015
View reviewed changes
web/viewer.js
@@ -291,7 +291,8 @@ var PDFViewerApplication = {
newScale = (newScale * DEFAULT_SCALE_DELTA).toFixed(2);
newScale = Math.ceil(newScale * 10) / 10;
newScale = Math.min(MAX_SCALE, newScale);
} while (--ticks && newScale < MAX_SCALE);
--ticks;
} while (ticks >= 0 && newScale < MAX_SCALE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that changing these lines to } while (--ticks > 0 && newScale < MAX_SCALE); should achieve what you want here (similar below).

@xlc xlc force-pushed the fix_scroll_zoom branch from f573794 to ed540a8 Compare May 14, 2015 22:17
@xlc
Copy link
Contributor Author

xlc commented May 14, 2015

@Snuffleupagus Fixed. The actual issue is the tick count is not always integer, so --ticks will reduce it to negative value.

@Snuffleupagus
Copy link
Collaborator

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/efa0c0abda89763/output.txt

@Snuffleupagus Snuffleupagus self-assigned this May 14, 2015
@pdfjsbot
Copy link

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/efa0c0abda89763/output.txt

Total script time: 0.95 mins

Published

Snuffleupagus added a commit that referenced this pull request May 14, 2015
@Snuffleupagus
Merge pull request #6014 from NotablePDF/fix_scroll_zoom
9deae0e 
Fix scroll to zoom jump between 25% and 1000%
@Snuffleupagus Snuffleupagus merged commit 9deae0e into mozilla:master May 14, 2015
@Snuffleupagus
Copy link
Collaborator

Thanks for the patch!

@Snuffleupagus Snuffleupagus mentioned this pull request May 14, 2015
Closed
@xlc xlc deleted the fix_scroll_zoom branch May 14, 2015 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Snuffleupagus Snuffleupagus

Labels
viewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ctrl + scroll to zoom jump from 25% to 1000%
3 participants
@xlc @Snuffleupagus @pdfjsbot