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

Allow to set up a hook at the end of the initialization (bug 1881319) #17709

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

calixteman
Copy link
Contributor

With this patch it's now possible to be sure to add a listener (for example for the "pagerendered" event) before the pdf is set.

With this patch it's now possible to be sure to add a listener
(for example for the "pagerendered" event) before the pdf is set.
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

Is it really not possible utilize an existing event, or perhaps even the PDFViewerApplication.initializedPromise promise, rather than having to add all of this new complexity here?
Also, in case it's helpful, please keep in mind that you can determine if tests are running in mozilla-central via the isInAutomation-property which is already available in the viewer through AppOptions.

All-in-all this code unfortunately does not feel all that readable/maintainable, and looking at the description here and in Bugzilla it still feels a bit like the "bigger picture" isn't entirely clear to me!?

@calixteman
Copy link
Contributor Author

The goal is to have some tests for the snap package of Firefox and they're using Selenium.
The test takes an image from the canvas and then compare it to a reference: the problem is to know when the canvas is ready.
So the idea is to set a listener for pagerendered but afaik there is no guarantee that the listener will be set before the event is triggered, hence this patch.
That said, if you've a better/simpler idea to achieve the same, please share it.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Feb 22, 2024

The goal is to have some tests for the snap package of Firefox and they're using Selenium.

How is the PDF viewer being loaded in this case, because it might be possible to work-around that?

Anyway, could we at least avoid adding a new Preference for this and keep it as "just" an option?
If we change this event to be dispatched in all builds, could the Selenium-tests listen for "webviewerloaded"[1] and then call e.g. window.PDFViewerApplicationOptions.set("hasInitializationHook", true) from that event listener instead?

Edit: Another possibility to avoid the Preference, assuming that isInAutomation === true when the Selenium-tests run, might be to just use the "initializationhook" hash-parameter there as well.


[1] Since that event is dispatched before the viewer-initialization runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants