Last Comment Bug 631500 - Let openUILink accept an object with named parameters
: Let openUILink accept an object with named parameters
Status: RESOLVED FIXED
[qa?]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 14
Assigned To: Dão Gottwald [:dao]
:
:
Mentors:
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)
anthony.s.hughes: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
14+
fixed


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

Description Dão Gottwald [:dao] 2011-02-04 03:55:23 PST
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.
Comment 1 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-06-07 17:42:08 PDT
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.
Comment 2 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 Phil Ringnalda (:philor) 2012-03-18 13:11:02 PDT
https://hg.mozilla.org/mozilla-central/rev/35c61da53e1a
Comment 5 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 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-05-10 18:08:18 PDT
Do you mean backport tracking? We don't want to back this out anywhere :)
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 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 :Gavin Sharp [email: gavin@gavinsharp.com] 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 :Gavin Sharp [email: gavin@gavinsharp.com] 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 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 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-29 09:33:15 PDT
https://hg.mozilla.org/releases/mozilla-esr10/rev/07ebb24b04ab
Comment 12 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.