Last Comment Bug 753308 - Call openUILink with named parameters
: Call openUILink with named parameters
Status: RESOLVED FIXED
[good first bug][mentor=dao][lang=js]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 15
Assigned To: Raymond Lee [:raymondlee]
:
Mentors:
Depends on: 631500
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-09 07:03 PDT by Dão Gottwald [:dao]
Modified: 2012-10-09 23:14 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (2.62 KB, patch)
2012-05-10 19:15 PDT, Raymond Lee [:raymondlee]
dao+bmo: feedback+
Details | Diff | Review
v2 (2.30 KB, patch)
2012-05-11 10:58 PDT, Raymond Lee [:raymondlee]
dao+bmo: review+
Details | Diff | Review

Description Dão Gottwald [:dao] 2012-05-09 07:03:04 PDT
http://mxr.mozilla.org/mozilla-central/search?string=openUILink%28&filter=,%20%28true|false|null%29

openUILink(url, event, x, y, z, ...);

should be replaced with:

openUILink(url, event, {
           ignoreButton: x,
           ignoreAlt: y,
           allowThirdPartyFixup: z,
           ...});
Comment 1 Raymond Lee [:raymondlee] 2012-05-10 19:15:03 PDT
Created attachment 623009 [details] [diff] [review]
v1
Comment 2 Dão Gottwald [:dao] 2012-05-11 00:54:00 PDT
Comment on attachment 623009 [details] [diff] [review]
v1

>--- a/browser/base/content/browser-places.js
>+++ b/browser/base/content/browser-places.js

>   _onCommand: function HM__onCommand(aEvent) {
>     let placesNode = aEvent.target._placesNode;
>     if (placesNode) {
>       PlacesUIUtils.markPageAsTyped(placesNode.uri);
>-      openUILink(placesNode.uri, aEvent, false, true);
>+      openUILink(placesNode.uri, aEvent, { ignoreButton: false,
>+                                           ignoreAlt: true });

"ignoreButton: false" can be removed, it's the default behavior

>--- a/browser/base/content/browser.js
>+++ b/browser/base/content/browser.js

>   loadFeed: function(href, event) {
>     var feeds = gBrowser.selectedBrowser.feeds;
>     try {
>-      openUILink(href, event, false, true, false, null);
>+      openUILink(href, event, { ignoreButton: false,
>+                                ignoreAlt: true,
>+                                allowThirdPartyFixup: false,
>+                                postData: null });
>     }

ditto for ignoreButton, allowThirdPartyFixup and postData.

>--- a/browser/base/content/pageinfo/feeds.js
>+++ b/browser/base/content/pageinfo/feeds.js

> function onSubscribeFeed()
> {
>   var listbox = document.getElementById("feedListbox");
>-  openUILink(listbox.selectedItem.getAttribute("feedURL"),
>-             null, false, true, false, null);
>+  openUILink(listbox.selectedItem.getAttribute("feedURL"), null, {
>+             ignoreButton: false,
>+             ignoreAlt: true,
>+             allowThirdPartyFixup: false,
>+             postData: null });
> }

Passing a null event seems strange. It looks like this should be using openUILinkIn.
Comment 3 Raymond Lee [:raymondlee] 2012-05-11 10:58:54 PDT
Created attachment 623220 [details] [diff] [review]
v2
Comment 5 Matt Brubeck (:mbrubeck) 2012-05-13 17:40:30 PDT
https://hg.mozilla.org/mozilla-central/rev/c216e50bdc0d

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