451264 - Need the ability to run reftest-wait and reftest-print together

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[cmtalbert]

Currently, you cannot run an asynchronous reftest-print test.  We need this for testing print-selection area testing as well as downloadable font testing.  

The basic idea would be to enable you to specify both strings in the reftest document's class attribute, then you would choose remove the reftest-wait string when you were ready for the reftest to proceed.

== Expected ==
You would be able to run an asynchronous reftest

== Actual ==
The test being run is not formatted using the reftest-print formatting.  Instead, it is formatted as a normal reftest and therefore fails the canvas comparison.

Comment 1

[cmtalbert]

Attachment: A WIP patch for this support

Here is a work in progress patch.  However, I think that the setPageMode[1] call causes the window.onload function defined in the test to be somehow lost.  Therefore, the "reftest-wait" value is never removed from the class attribute.  

I'm not sure if the setPageMode destroys all previous event handlers held by that page, or if the setPageMode executes its own "onload" handler that the test should be using instead of window.onload.

So, my basic question: does the test need to be changed in order to get around this problem, or is there code that needs to be added to reftest.js after the setPageMode to re-enable this event, or does something need to be change in the setPageMode implementation to not destroy this event handler?

[1]: The location of the setPageMode call in the original code: http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest.js#394

Comment 2

[cmtalbert]

Attachment: Patch that fixes the issue

Thanks to jst for his help in sorting this out.  Evidently, setPageMode turns off javascript in the content that it is converting to print preview mode.  Therefore, our javascript that removes the "reftest-wait" value never gets called.  

This fix factors out the print preview stuff into a function, then calls it once the reftest-wait value is removed.  The original functionality of reftest-print should not be changed at all.

Comment 3

[Akira Yoshihara]

Patch seems to work great.

Now I can execute asynchronous printing tests by the following procedures:

1. make sure a class 'reftest-wait' is on the root element by the moment the load event fires
2. when the test is ready, assign 'reftest-print' to the root element, instead of just removing the previous className 'reftest-wait'

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 334750 [details] [diff] [review]
Patch that fixes the issue

This looks fine, except I think test-async-print.html should start with the class attribute being class="reftest-wait reftest-print"; I don't think it makes sense to dynamically change whether it's reftest-print in the middle of the run.

r=dbaron with that

Comment 5

[cmtalbert]

ok patch checked in:
changeset:   18874:563df8c7a7e3
tag:         tip
user:        Clint Talbert <[e-mail]>
date:        Fri Sep 05 17:30:45 2008 -0700
summary:     bug 451264 allowing reftest-wait and reftest-print together r=dbaron