Let openUILink accept an object with named parameters

RESOLVED FIXED in Firefox -esr10

Status

()

Firefox
General
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 14
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox-esr1014+ fixed)

Details

(Whiteboard: [qa?])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
Created attachment 509728 [details] [diff] [review]
patch

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-
(Assignee)

Updated

6 years ago
Blocks: 734076
(Assignee)

Comment 2

6 years ago
Created attachment 606870 [details] [diff] [review]
patch v2

fixed the argument names
Attachment #509728 - Attachment is obsolete: true
Attachment #606870 - Flags: review?(gavin.sharp)
Attachment #606870 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 3

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/35c61da53e1a
Target Milestone: --- → Firefox 14
https://hg.mozilla.org/mozilla-central/rev/35c61da53e1a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Blocks: 753308
marking this for backout tracking for esr based on dep bug 734076
status-firefox-esr10: --- → affected
tracking-firefox-esr10: --- → 14+
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.
https://hg.mozilla.org/releases/mozilla-esr10/rev/07ebb24b04ab
status-firefox-esr10: affected → fixed
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.