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

Prepare the MOZCENTRAL viewer for receiving zoom events from the browser UI (bug 786674, bug 1177385) #10652

Merged
merged 1 commit into from
Mar 21, 2019

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Mar 19, 2019

This lays the necessary foundation for handling zoom events originating within the browser itself, rather than in the viewer. Please note that this will also require a follow-up patch to mozilla-central, such that the viewer is actually notified when zooming occurs. For example something like this: https://gist.github.com/Snuffleupagus/8d27775a3b18f1cd6cac88351edf4f7f

@Snuffleupagus
Copy link
Collaborator Author

/botio lint

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/9aa500be1df356a/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

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

Total script time: 0.98 mins

  • Lint: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/9aa500be1df356a/output.txt

Total script time: 2.99 mins

  • Lint: Passed

@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Mar 20, 2019

Unfortunately there isn't really a good way of testing this, short of applying the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=786674#c14 and then building Firefox itself, so I'm hoping that this can be accepted based on its high degree of similarity with the listenFindEvents block just above :-)

Copy link
Contributor

@timvandermeij timvandermeij left a comment

Choose a reason for hiding this comment

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

I'm fine with merging this if there is also an upstream reviewer. Perhaps @brendandahl can take a look at the Bugzilla patch, or find someone upstream that can?

web/firefoxcom.js Show resolved Hide resolved
web/firefoxcom.js Outdated Show resolved Hide resolved
web/firefoxcom.js Outdated Show resolved Hide resolved
@Snuffleupagus Snuffleupagus force-pushed the browser-find-events branch 2 times, most recently from 22b0009 to 13a9c21 Compare March 21, 2019 11:02
…owser UI (bug 786674, bug 1177385)

This lays the necessary foundation for handling zoom events originating within the browser itself, rather than in the viewer. Please note that this will also require a follow-up patch to `mozilla-central`, such that the viewer is actually notified when zooming occurs.
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/4ffe6c3fe400854/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/4ffe6c3fe400854/output.txt

Total script time: 1.86 mins

Published

@timvandermeij timvandermeij merged commit bce9ff7 into mozilla:master Mar 21, 2019
@timvandermeij
Copy link
Contributor

Thank you for the additional explanation! It really cleared up why we're doing this exactly.

@Snuffleupagus Snuffleupagus deleted the browser-find-events branch March 22, 2019 08:38
@Snuffleupagus
Copy link
Collaborator Author

Thank you for the additional explanation! It really cleared up why we're doing this exactly.

I should probably have included more information upfront, as this wasn't initially in the patch but seemed reasonable to add once I'd done a bit more testing.

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