The default bug view has changed. See this FAQ.

minor cleanup in animationPolling.js

RESOLVED FIXED in mozilla11

Status

()

Core
ImageLib
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla11
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
animationPolling.js calls a function "imageLoadCallback" here:
>26 function failTest ()
>27 {    imageLoadCallback();
http://mxr.mozilla.org/mozilla-central/source/image/test/mochitest/animationPolling.js?mark=27-27#26

but that function isn't defined anywhere, as shown by this search:
http://mxr.mozilla.org/mozilla-central/search?string=imageLoadCallback

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

Comment 1

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

Comment 2

5 years ago
Looks like this line was added in:
 http://hg.mozilla.org/mozilla-central/rev/47131206c09f
and that cset also removed the definition of "imageLoadCallback".
Blocks: 702093
(Assignee)

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
(Assignee)

Comment 4

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

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
(Assignee)

Updated

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+
(Assignee)

Updated

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

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+
(Assignee)

Comment 9

5 years ago
Landed patch 1 (I gave it a TryServer run overnight last night):
 https://hg.mozilla.org/integration/mozilla-inbound/rev/68f0d4831c48

I'll give patch 2 a TryServer sanity-check and then land it later today.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla11
(Assignee)

Comment 10

5 years ago
Landed patch 2:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/9b50bc5ef548
https://hg.mozilla.org/mozilla-central/rev/9b50bc5ef548
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/mozilla-central/rev/68f0d4831c48
You need to log in before you can comment on or make changes to this bug.