505905 - InstallTriggerGlobalInstallChrome should check the result of JS_ValueToECMAUint32

Status: RESOLVED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect)

Description

[timeless]

# xpinstall/src/nsJSInstallTriggerGlobal.cpp
    * line 414 -- JS_ValueToECMAUint32(cx, argv[0], &chromeType); 

everyone else does

Comment 1

[Reed Loden [:reed]]

Attachment: patch - v1

Let's join the party, then!

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 400518 [details] [diff] [review]
patch - v1

Looks ok to me, but I'm not a peer here

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 400518 [details] [diff] [review]
patch - v1

r=dveditz

Comment 4

[Reed Loden [:reed]]

http://hg.mozilla.org/mozilla-central/rev/5c5e2ca2066d

Comment 5

[Reed Loden [:reed]]

Backed this out as a possible cause of a talos Ts dirty profile regression, though I don't really believe it.

Comment 6

[Reed Loden [:reed]]

Relanded. Backout had no effect.

http://hg.mozilla.org/mozilla-central/rev/927485ca03ea

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 400518 [details] [diff] [review]
patch - v1

approval1.9.2 requests aren't currently being monitored, since we're nearing RC freeze and there are too many outstanding requests, so I'm clearing this request. Feel free to re-request approval if you are confident that it's worth drivers' time to consider whether this non-blocker needs to land for 1.9.2 at this stage.

Comment 8

[Dave Townsend [:mossop]]

Comment on attachment 400518 [details] [diff] [review]
patch - v1

Please provide a decent explanation of the risks vs. rewards of taking this fix at this late stage in the release before requesting approval. Why do we need to fix this bug in 1.9.2 when it has been present since Mozilla 1.7?