Closed Bug 451264 Opened 13 years ago Closed 13 years ago

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

Categories

(Testing :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: cmtalbert, Assigned: cmtalbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch A WIP patch for this support (obsolete) — Splinter Review
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
Assignee: nobody → ctalbert
Status: NEW → ASSIGNED
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.
Attachment #334708 - Attachment is obsolete: true
Attachment #334750 - Flags: review?
Attachment #334750 - Flags: review? → review?(dbaron)
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 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
Attachment #334750 - Flags: review?(dbaron) → review+
ok patch checked in:
changeset:   18874:563df8c7a7e3
tag:         tip
user:        Clint Talbert <ctalbert@mozilla.com>
date:        Fri Sep 05 17:30:45 2008 -0700
summary:     bug 451264 allowing reftest-wait and reftest-print together r=dbaron
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Component: New Frameworks → General
You need to log in before you can comment on or make changes to this bug.