Last Comment Bug 712649 - Components.utils.getWeakReference(null) should fail silently (as it used to do)
: Components.utils.getWeakReference(null) should fail silently (as it used to do)
Status: VERIFIED FIXED
[qa!] [testday-20120203]
: addon-compat, dev-doc-complete
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: 11 Branch
: All All
: -- normal (vote)
: mozilla12
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 708330
  Show dependency treegraph
 
Reported: 2011-12-21 07:50 PST by Giorgio Maone [:mao]
Modified: 2012-04-24 05:46 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
unaffected
unaffected
+
verified
verified


Attachments
Components.utils.getWeakReference(null) should fail silently; (1.88 KB, patch)
2011-12-21 07:59 PST, :Ms2ger (⌚ UTC+1/+2)
bobbyholley: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Giorgio Maone [:mao] 2011-12-21 07:50:23 PST
In 11 and above Components.utils.getWeakReference(null) throws NS_ERROR_FAILURE.
It used to fail silently instead, so it's probably breaking some clients (NoScript, for instance).
Also, since the value held by a weak reference will eventually be null anyway, allowing clients pass null may be seen as more consistent.
Comment 1 :Ms2ger (⌚ UTC+1/+2) 2011-12-21 07:59:03 PST
Created attachment 583499 [details] [diff] [review]
Components.utils.getWeakReference(null) should fail silently;

I think it makes sense to allow this, but we should probably keep propagating other exceptions.
Comment 2 Bobby Holley (:bholley) (busy with Stylo) 2011-12-21 15:14:29 PST
Comment on attachment 583499 [details] [diff] [review]
Components.utils.getWeakReference(null) should fail silently;

Looks good. r=bholley

We should get this in for 11 too, otherwise it sounds like we'll break extensions.
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2011-12-24 01:24:44 PST
https://hg.mozilla.org/mozilla-central/rev/834bfdd83f70
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2011-12-24 01:25:56 PST
Comment on attachment 583499 [details] [diff] [review]
Components.utils.getWeakReference(null) should fail silently;

This is a regression from bug 708330, which broke NoScript. The patch should be safe.
Comment 5 Alex Keybl [:akeybl] 2011-12-26 11:19:04 PST
Comment on attachment 583499 [details] [diff] [review]
Components.utils.getWeakReference(null) should fail silently;

[Triage Comment]
Approved for Aurora given the risk of regressing add-ons.
Comment 6 :Ms2ger (⌚ UTC+1/+2) 2011-12-28 02:47:51 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/bf34343622f4
Comment 7 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-02-01 13:33:37 PST
Is there something QA can do to verify this fix? possibly involving NoScript?
Comment 8 :Ms2ger (⌚ UTC+1/+2) 2012-02-02 09:49:44 PST
I believe NoScript already works around this bug.
Comment 9 Giorgio Maone [:mao] 2012-02-02 12:29:08 PST
(In reply to Ms2ger from comment #8)
> I believe NoScript already works around this bug.

Yes it does.

You can verify by running the following in the Error Console or in a chrome-scoped scratchpad:

try { alert("OK (" + Components.utils.getWeakReference(null) + ")") } catch(e) { alert("FAIL") }
Comment 10 Virgil Dicu [:virgil] [QA] 2012-02-03 07:04:54 PST
Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0

Verified using guideline in comment 9 on Ubuntu 11.10, Mac OS 10.6, Windows XP (Firefox 11 beta 1). No failures.
All resulted in the following message in the Error Console:
"OK ([xpconnect wrapped xpcIJSWeakReference])"
Comment 11 Virgil Dicu [:virgil] [QA] 2012-03-30 05:48:03 PDT
Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0) Gecko/20100101 Firefox/12.0

Marking verified after checking on Firefox 12 beta 3 with the same guidelines from comment 9. 
Result: "OK ([xpconnect wrapped xpcIJSWeakReference])"
Comment 12 Eric Shepherd [:sheppy] 2012-04-24 05:46:24 PDT
Documentation updated:

https://developer.mozilla.org/en/Components.utils.getWeakReference

Listed on Firefox 12 for developers.

Note You need to log in before you can comment on or make changes to this bug.