Components.utils.getWeakReference(null) should fail silently (as it used to do)

VERIFIED FIXED in Firefox 11

Status

()

defect
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: mao, Assigned: Ms2ger)

Tracking

({addon-compat, dev-doc-complete})

11 Branch
mozilla12
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox8 unaffected, firefox9 unaffected, firefox10 unaffected, firefox11+ verified, firefox12 verified)

Details

(Whiteboard: [qa!] [testday-20120203])

Attachments

(1 attachment)

Reporter

Description

8 years ago
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.
Reporter

Updated

8 years ago
Blocks: 708330
Assignee

Comment 1

8 years ago
I think it makes sense to allow this, but we should probably keep propagating other exceptions.
Attachment #583499 - Flags: review?(bobbyholley+bmo)
Assignee

Updated

8 years ago
Assignee: nobody → Ms2ger
Target Milestone: --- → mozilla11
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.
Attachment #583499 - Flags: review?(bobbyholley+bmo) → review+
Target Milestone: mozilla11 → ---
Version: Trunk → 11 Branch
Assignee

Comment 3

8 years ago
https://hg.mozilla.org/mozilla-central/rev/834bfdd83f70
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Assignee

Comment 4

8 years ago
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.
Attachment #583499 - Flags: approval-mozilla-aurora?
Assignee

Updated

8 years ago
Keywords: dev-doc-needed
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.
Attachment #583499 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: addon-compat
Is there something QA can do to verify this fix? possibly involving NoScript?
Whiteboard: [qa?]
Assignee

Comment 8

7 years ago
I believe NoScript already works around this bug.
Reporter

Comment 9

7 years ago
(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") }
Whiteboard: [qa?] → [qa+]
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])"
Whiteboard: [qa+] → [qa+] [testday-20120203] [qa!:11]
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])"
Status: RESOLVED → VERIFIED
Whiteboard: [qa+] [testday-20120203] [qa!:11] → [qa!] [testday-20120203]
Documentation updated:

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

Listed on Firefox 12 for developers.
You need to log in before you can comment on or make changes to this bug.