Last Comment Bug 631500 - Let openUILink accept an object with named parameters
: Let openUILink accept an object with named parameters
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
-- normal (vote)
: Firefox 14
Assigned To: Dão Gottwald [:dao]
Depends on:
Blocks: CVE-2012-1966 753308
  Show dependency treegraph
Reported: 2011-02-04 03:55 PST by Dão Gottwald [:dao]
Modified: 2012-10-09 23:14 PDT (History)
5 users (show) in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (1.65 KB, patch)
2011-02-04 03:55 PST, Dão Gottwald [:dao] review-
Details | Diff | Splinter Review
patch v2 (1.76 KB, patch)
2012-03-17 08:31 PDT, Dão Gottwald [:dao] review+
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description User image Dão Gottwald [:dao] 2011-02-04 03:55:23 PST
Created attachment 509728 [details] [diff] [review]

As opposed to:

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

This doesn't work:

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

The attached patch fixes that.
Comment 1 User image :Gavin Sharp [email:] 2011-06-07 17:42:08 PDT
Comment on attachment 509728 [details] [diff] [review]

>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.
Comment 2 User image Dão Gottwald [:dao] 2012-03-17 08:31:11 PDT
Created attachment 606870 [details] [diff] [review]
patch v2

fixed the argument names
Comment 4 User image Phil Ringnalda (:philor) 2012-03-18 13:11:02 PDT
Comment 5 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-05-10 16:23:03 PDT
marking this for backout tracking for esr based on dep bug 734076
Comment 6 User image :Gavin Sharp [email:] 2012-05-10 18:08:18 PDT
Do you mean backport tracking? We don't want to back this out anywhere :)
Comment 7 User image :Gavin Sharp [email:] 2012-06-22 19:23:34 PDT
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
Comment 8 User image :Gavin Sharp [email:] 2012-06-22 19:32:57 PDT
Comment on attachment 606870 [details] [diff] [review]
patch v2

Could use this on beta too, for the same reasons.
Comment 9 User image :Gavin Sharp [email:] 2012-06-22 19:35:34 PDT
Comment on attachment 606870 [details] [diff] [review]
patch v2

(nevermind, I was confused, this is already on beta)
Comment 10 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-06-29 09:21:23 PDT
blocking bug 734076 which is tracked for ESR, please go ahead and land.
Comment 11 User image :Gavin Sharp [email:] 2012-06-29 09:33:15 PDT
Comment 12 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-08-14 14:21:29 PDT
Does this need a test?

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