Skip to content

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

Closed
calixteman wants to merge 1 commit into
mozilla:masterfrom
calixteman:test_init_hook
Closed

Allow to set up a hook at the end of the initialization (bug 1881319)#17709
calixteman wants to merge 1 commit into
mozilla:masterfrom
calixteman:test_init_hook

Conversation

@calixteman

Copy link
Copy Markdown
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.

@Snuffleupagus Snuffleupagus left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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

Snuffleupagus commented Feb 22, 2024

Copy link
Copy Markdown
Collaborator

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.

@timvandermeij

Copy link
Copy Markdown
Collaborator

Closing in favor of #18617 which only requires changing test code and overall seems safer/simpler.

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.

3 participants