Closed Bug 631500 Opened 11 years ago Closed 9 years ago

Let openUILink accept an object with named parameters

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 14
Tracking Status
firefox-esr10 14+ fixed

People

(Reporter: dao, Assigned: dao)

References

Details

(Whiteboard: [qa?])

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
As opposed to:

openUILinkIn(url, whereToOpenLink(event), { relatedToCurrent: true });

This doesn't work:

openUILink(url, event, { relatedToCurrent: true });

The attached patch fixes that.
Attachment #509728 - Flags: review?(gavin.sharp)
Comment on attachment 509728 [details] [diff] [review]
patch

>diff --git a/browser/base/content/utilityOverlay.js b/browser/base/content/utilityOverlay.js

>+function openUILink(url, e, ignoreButton, ignoreAlt, allowKeywordFixup, postData, referrerUrl) {

>+    params = {
>+      allowThirdPartyFixup: allowThirdPartyFixup,

>+      referrerURI: referrerURI

these don't match up (allowKeywordFixup and referrerUrl)

Should have a test that would've caught this if there isn't already.
Attachment #509728 - Flags: review?(gavin.sharp) → review-
Attached patch patch v2Splinter Review
fixed the argument names
Attachment #509728 - Attachment is obsolete: true
Attachment #606870 - Flags: review?(gavin.sharp)
Attachment #606870 - Flags: review?(gavin.sharp) → review+
https://hg.mozilla.org/mozilla-central/rev/35c61da53e1a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Blocks: 753308
marking this for backout tracking for esr based on dep bug 734076
Do you mean backport tracking? We don't want to back this out anywhere :)
Comment on attachment 606870 [details] [diff] [review]
patch v2

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: blocks bug 734076
User impact if declined: 
Fix Landed on Version: landed with 14, no known regressions
Risk to taking this patch (and alternatives if risky): purely additional, backwards-compatible change, so should not cause any trouble
String or UUID changes made by this patch: none
Attachment #606870 - Flags: approval-mozilla-esr10?
Comment on attachment 606870 [details] [diff] [review]
patch v2

Could use this on beta too, for the same reasons.
Attachment #606870 - Flags: approval-mozilla-beta?
Comment on attachment 606870 [details] [diff] [review]
patch v2

(nevermind, I was confused, this is already on beta)
Attachment #606870 - Flags: approval-mozilla-beta?
Attachment #606870 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
blocking bug 734076 which is tracked for ESR, please go ahead and land.
Does this need a test?
Flags: in-testsuite?
Whiteboard: [qa?]
You need to log in before you can comment on or make changes to this bug.