The default bug view has changed. See this FAQ.

minor cleanup in animationPolling.js

RESOLVED FIXED in mozilla11



5 years ago
5 years ago


(Reporter: dholbert, Assigned: dholbert)



Firefox Tracking Flags

(Not tracked)




(2 attachments)



5 years ago
animationPolling.js calls a function "imageLoadCallback" here:
>26 function failTest ()
>27 {    imageLoadCallback();

but that function isn't defined anywhere, as shown by this search:

Looks like a typo.  Scott, should it be removed, or is there for a reason and we need to make it actually do something?

Comment 1

5 years ago
er I meant "or is _it_ there for a reason"

Comment 2

5 years ago
Looks like this line was added in:
and that cset also removed the definition of "imageLoadCallback".
Blocks: 702093

Comment 3

5 years ago
Taking - I noticed a few other things in animationPolling that could use a tweak, too - I'll just post 'em in a combined patch here.
Assignee: nobody → dholbert+bmo

Comment 4

5 years ago
Created attachment 579917 [details] [diff] [review]
patch 1 v1
Attachment #579917 - Flags: review?(sjohnson)

Comment 5

5 years ago
Summary of changes:
 - Remove the invalid "imageLoadCallback()" call. (I verified that, in the case of test failure, we currently throw exceptions due to imageLoadCallback not existing -- but with that call removed, we correctly report the failure as "test still doesn't match ref after call #N etc")
 - Removed space between function-name and () in a few spots
 - Fixed some greater-than-80-characters lines (mostly "compareSnapshots" calls).
 - Changed a setTimeout(..., 10) to executeSoon (which does the same thing, basically, without a magic number)

 - changed a repeated setTimeout(..., 20) to use currentTest.pollFreq
 - fixed comment & variable-name in clause where "reference div" isn't actually a div


5 years ago
OS: Linux → All
Hardware: x86_64 → All
Comment on attachment 579917 [details] [diff] [review]
patch 1 v1

Review of attachment 579917 [details] [diff] [review]:

Wow, thanks for catching some of these half-line typos. I can't believe it was passing with these.
Attachment #579917 - Flags: review?(sjohnson) → review+


5 years ago
Summary: animationPolling.js calls undefined function "imageLoadCallback()" → minor cleanup in animationPolling.js

Comment 7

5 years ago
Created attachment 580143 [details] [diff] [review]
patch 2 v1: turn off debug output (w/ boolean global), and add emacs mode line

Currently animationPolling triggers a lot of debugging screenshots to be printed to mochitest logs.

This patch turns that off (but adds a boolean var so we can toggle it on again if necessary for debugging an issue).

I also added an emacs modeline at the top (copypasted from imgutils.js), so that emacs knows to indent by 2 spaces instead of 4.
Attachment #580143 - Flags: review?(sjohnson)
Comment on attachment 580143 [details] [diff] [review]
patch 2 v1: turn off debug output (w/ boolean global), and add emacs mode line

dholbert and I discussed this on IRC. Thanks for doing this, dholbert! r=me.
Attachment #580143 - Flags: review?(sjohnson) → review+

Comment 9

5 years ago
Landed patch 1 (I gave it a TryServer run overnight last night):

I'll give patch 2 a TryServer sanity-check and then land it later today.
Target Milestone: --- → mozilla11

Comment 10

5 years ago
Landed patch 2:
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.