807210 - JavaScript strict warning: chrome://pageloader/content/pageloader.js, line 275: assignment to undeclared variable gPaintHandler

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

Message encountered when running talos test tdhtmlr

Comment 1

[Jeff Hammel]

Presuming this isn't platform specific

Comment 2

[Jeff Hammel]

ABICT, gPaintHandler isn't used at all.

Comment 3

[Jeff Hammel]

Is this supposed to be gPaintListener?

Comment 4

[Jeff Hammel]

Attachment: just remove it

This just removes the thing and changes the license block.  If it in fact should be gPaintListener then this patch is wrong ;)

Comment 5

[Jeff Hammel]

introduced via http://hg.mozilla.org/build/talos/diff/0b38ce037298/chrome/pageloader.js

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 681270 [details] [diff] [review]
just remove it

Review of attachment 681270 [details] [diff] [review]:
-----------------------------------------------------------------

I am concerned about this, the intention was to mark gPaintHandler as false so that we can change a global state.  We need to know if a page is loaded and if mozAfterPaint is true, then we have to wait for gPaintHandler to be set to True.  Where is that set now?

Comment 7

[Jeff Hammel]

Current gPaintHandler appears only in the two places set in the patch.  Should these be gPaintListener instead?  That is defined at module scope and used several other places in the file.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

ahh, we need to change s/gPaintHandler/gPaintListener/, then things should work as expected.

Comment 9

[Jeff Hammel]

Attachment: gPaintHandler -> gPaintListener

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 681540 [details] [diff] [review]
gPaintHandler -> gPaintListener

Review of attachment 681540 [details] [diff] [review]:
-----------------------------------------------------------------

awesome.

Comment 11

[Jeff Hammel]

What sort of testing should i do before I push?

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

I would run all tests on a single platform.

Comment 13

[Jeff Hammel]

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=eff502e45f8d

Comment 14

[Mozilla RelEng Bot]

Try run for eff502e45f8d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=eff502e45f8d
Results (out of 7 total builds):
    success: 7
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-eff502e45f8d

Comment 15

[Jeff Hammel]

pushed: http://hg.mozilla.org/build/talos/rev/9a88382581a3