336452 - Improve search-engine confirmation dialog

Status: RESOLVED FIXED

(Firefox :: Search, enhancement, P1)

Description

[Pam Greene]

Improve dialog that asks for confirmation when adding a search engine with the JS function.

+------------------------------------------------------+
|                                                      |
|  /-\     Add %s to drop-down list of search engines? |
|  | |                                                 |
|  \-/ V   o Start using this one right now.           |
|     \                                                |
|                                         [No]  [Yes]  |
+------------------------------------------------------+

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Also, if "yes" is selected, focus should be given to the search field.

Comment 2

[Pam Greene]

If "yes" is selected, or only if the checkbox is checked?

What if loading the engine's information takes a little while?  We can't select the new engine until it's loaded, but that might be a long enough delay from when this dialog is shown (perhaps even seconds later?) that it would be disruptive to move focus.

Comment 3

[Pam Greene]

Attachment: WIP for UI and string review

Comment 4

[Pam Greene]

Attachment: Dialog box when tentative title is known

This dialog comes up when the engine has a known, content-supplied title (that is, when our old window.sidebar.addSearchEngine() function is called, like on addons.mozilla.org).

Comment 5

[Pam Greene]

Attachment: Dialog box when no title is known

This dialog comes up when all we have is a URI for the engine description file, which happens when someone's using the new IE7-like API.

Comment 6

[Pam Greene]

This is a work in progress. It could be used as-is, but I'd like to make additional improvements.  Also, this patch incorporates all of bug 337780 right now, to be split apart once that bug is reviewed and landed.

For now I'm looking for UI and string review; see the two screenshots.  I think this is better than the current dialog, but there are two additional improvements I'd like to make:

First, I'd like to load the full engine description, with a few seconds' timeout, before showing the dialog, so we always have a good title to display.  That may involve another string or two (something like "Loading search-engine information..." for an interstitial dialog before the real one is available?).

Second, I'd like to show the big magnifying-glass icon in the dialog.  That turns out to be harder than I thought it'd be, so although I have a pretty good idea of how to do it, I'd like to defer it until the basic fix (this patch) and the first improvement described above are done.

Comment 7

[Joe Hughes]

I'm curious to see what happens in the no-title case where the URL is long...

Comment 8

[Pam Greene]

Attachment: Long URL

On Mac at least, the dialog box simply gets wider, even if (as in this screenshot) that means it's wider than its parent window.  I assume if the dialog were wider than the entire screen, the result would be bad.  I'd call the lack of wrapping a bug in the prompt service, but I don't think it'll come up often, and the misbehavior is tolerable for all but the most pathological cases.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #6)
> For now I'm looking for UI and string review; see the two screenshots.  I 

Coming up ...

> First, I'd like to load the full engine description, with a few seconds'
> timeout, before showing the dialog, so we always have a good title to display. 
> That may involve another string or two (something like "Loading search-engine
> information..." for an interstitial dialog before the real one is available?).

Great idea, Pam. I would suggest: "Getting name of search engine ...", and provide a (Cancel) button during this interstitial. Also, some logic to ensure that the user doesn't see a UI flicker or flash in case the search engine comes down so quickly that the interstitial is immediately replaced will be required. We might want to track this in a separate bug.

> Second, I'd like to show the big magnifying-glass icon in the dialog.  That
> turns out to be harder than I thought it'd be, so although I have a pretty 

Again, sounds good. I'd recommend that until final art is provided, you use the magnifying glass from the Tango project:

http://tango.freedesktop.org/static/cvs/tango-icon-theme/32x32/actions/system-search.png
or http://tango.freedesktop.org/static/cvs/tango-icon-theme/scalable/actions/system-search.svg

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 225458 [details] [diff] [review]
WIP for UI and string review

As I look at the screenshots, the term "drop-down" seems needlessly tecchie to me. IIRC, we added it to give people a strong cue to the fact that the engine will be available in the search bar.

A less techhie way of saying this would be: 

"Add %s to the list of engines available in the search bar?"

And re: comment 2. Right, only if the checkbox is selected. See, you knew what I meant. :)

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

(sorry for bugspam)

Ooh, also, the buttons should be more informative:

     ( Cancel )  ( Add )

This would also mean that the interstitial -> final dialog we were talking about would just add an "Add" button :)

Comment 12

[Pam Greene]

Attachment: WIP patch addressing Beltzner's comments

Still a work in progress, but here's a patch incorporating the UI comments.  It could be landed in this form (after review) to get all but one of the strings in if need be.

Do we want to show an interstitial dialog?  Or, since the user ought to have clicked on a link or button to cause the search-engine load to happen, maybe it should behave the same way clicking an anchor link does?  (Change the cursor to the spinning "loading" indicator, run the throbber, all that; and let ESC or the platform equivalent cancel the action, just like when loading a new page.)

Comment 13

[Pam Greene]

Attachment: New screenshot with tentative title known

The other cases (no title, short and long URLs) have changed analogously, of course.

Comment 14

[Joe Hughes]

(In reply to comment #12)

> Do we want to show an interstitial dialog?  Or, since the user ought to have
> clicked on a link or button to cause the search-engine load to happen, maybe it
> should behave the same way clicking an anchor link does?  (Change the cursor to
> the spinning "loading" indicator, run the throbber, all that; and let ESC or
> the platform equivalent cancel the action, just like when loading a new page.)
> 

I think it'd be better to show an interstitial immediately.  Otherwise, you run the risk of the user switching tabs and then getting a dialog that's tied to an action that they performed in a different tab, which could be confusing...

Comment 15

[Pam Greene]

About like a restricted-site login dialog, I guess.  And, true, those are a bit annoying when they steal focus after you've switched to something else, but they do stay attached to their own tabs.

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 225648 [details] [diff] [review]
WIP patch addressing Beltzner's comments

Thanks, Pam.

re:interstitial. Gavin tells me that searchplugins are 1-2KB. Assuming that we load that first, before the icon for the searchplugin, how long could the load and parse operation take? If it's under 5s, I don't think an interstitial is necessary.

Comment 17

[Jesse Ruderman]

The name of the search engine should be set apart from the rest of the dialog text in some way.  Putting quotes around it might probably be enough, assuming we don't have to worry about attacks like in bug 253942 for this dialog.

I'm worried that having "Start using it right away" checked by default will encourage abuse.  How about opening the search engine list dropdown automatically after clicking "Add" instead?

Does anything prevent me from making a fake "Google" and tricking users into installing and using it by throwing up a "Add 'Google' to the list of search engines available in the search bar?" dialog randomly?

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #17)
> The name of the search engine should be set apart from the rest of the dialog
> text in some way.  Putting quotes around it might probably be enough, assuming
> we don't have to worry about attacks like in bug 253942 for this dialog.

If you think it's important from a security perspective, then I'm OK with it. Most searchplugins will use a capital letter, so the string would be "Add Wikipedia to the list of ..." which I think reads fine from a UE perspective without the extra quotes.

> I'm worried that having "Start using it right away" checked by default will
> encourage abuse.  How about opening the search engine list dropdown
> automatically after clicking "Add" instead?

We considered this, and I thought it would be pretty obnoxious since it means clicking add ends up changing the state of the UI even in the case where the checkbox isn't selected. I'm not sure what you mean by "abuse", though. How about if the checkbox is checked and the user clicks yes, then instead of simply putting focus in the seach box, we put focus there *and* expand the drop-down?

> Does anything prevent me from making a fake "Google" and tricking users into
> installing and using it by throwing up a "Add 'Google' to the list of search
> engines available in the search bar?" dialog randomly?

Yes, bug 340122.

Comment 19

[Jesse Ruderman]

> Yes, bug 340122.

Including homographs and the like?

Comment 20

[Pam Greene]

(In reply to comment #16)
> re:interstitial. Gavin tells me that searchplugins are 1-2KB. Assuming that we
> load that first, before the icon for the searchplugin, how long could the load
> and parse operation take? If it's under 5s, I don't think an interstitial is
> necessary.

I don't know that much about the networking internals, but there can be significant latency in getting any response back from a random server, what with DNS and flaky backbone routers and all the other uncontrollable network conditions.  I'd expect a slow response to be rare, but not unknown.

Comment 21

[Pam Greene]

(In reply to comment #18)
> How
> about if the checkbox is checked and the user clicks yes, then instead of
> simply putting focus in the seach box, we put focus there *and* expand the
> drop-down?

If the checkbox is checked and the user clicks yes, the new engine's icon appears in the searchbox.  Opening the drop-down seems redundant and intrusive.

On the other hand, it wouldn't be a big deal to simply have the checkbox default to unchecked.  Would that allay your concerns, Jesse?

> > Does anything prevent me from making a fake "Google" and tricking users into
> > installing and using it by throwing up a "Add 'Google' to the list of search
> > engines available in the search bar?" dialog randomly?
> 
> Yes, bug 340122.

Although 'Google' isn't the same as 'Goog-one-e' or even 'Google Search', and none of them helps if I don't already have Google in my list.  I'm inclined to put the search description's source URL back into the dialog: it's ugly, but without it even a savvy user has to crawl through a page's JS to detect a spoof.

Comment 22

[Jesse Ruderman]

Having the checkbox default to unchecked and including a URL would take care of most of my security concerns.

Comment 23

[Pam Greene]

Splitting the two additional improvements described in comment #6 into bug 341668 and bug 361669, respectively.

Comment 24

[Pam Greene]

Sorry, that's bug 341669 for the second one, not 361669.

Comment 25

[Pam Greene]

Attachment: WIP patch addressing Jesse's security concerns

Beltzner: Is the UI still acceptable?
Jesse: Is it sufficiently secure?

(I'll get a review of the actual code once it's a patch intended for checkin.)

Still a WIP; see comment #12.

Comment 26

[Pam Greene]

Attachment: Screenshot, tentative title known

Comment 27

[Pam Greene]

Attachment: Screenshot, no title

Comment 28

[Jesse Ruderman]

I can see the argument that the problems with a long search engine name or URL (making the dialog wider than the screen and/or pushing buttons off of the screen) are a bug in the prompt service.  Requiring each user of the prompt service to avoid triggering that kind of problem does seem wrong.  Do we have a bug on that?  On the other hand, the dialog service might not know what text to truncate and/or when to give up and pretend the user hit cancel.

An alternative to "no title" is to have both the name and the URL be below the question:

   Add a new engine to the list of engines available...?

   Name: Webster
   URL: http://addons.mozilla.org/...

I think having the title inline in the paragraph is ok, though.

A bit offtopic: Search engines from addons.mozilla.org should be served over https, not http.

Comment 29

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #28)
> A bit offtopic: Search engines from addons.mozilla.org should be served over
> https, not http.

Unfortunately, versions prior to 1.5.0.4 are afflicted by bug 267487, so that's not really an option (see also bug 335602).

Comment 30

[Pam Greene]

(In reply to comment #28)
> I can see the argument that the problems with a long search engine name or URL
> (making the dialog wider than the screen and/or pushing buttons off of the
> screen) are a bug in the prompt service.  Requiring each user of the prompt
> service to avoid triggering that kind of problem does seem wrong.  Do we have a
> bug on that?  On the other hand, the dialog service might not know what text to
> truncate and/or when to give up and pretend the user hit cancel.

Ideally, the dialog would get taller once it couldn't fit on the screen (or window) any wider, and any long text would wrap at spaces, hyphens, or wherever it had to, in order of preference.

A bit off-topic: Search engines from addons.mozilla.org should pass better names to the JS now that they'll be displayed in the dialogs.

Comment 31

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #25)
> Beltzner: Is the UI still acceptable?

IMO, no. Adding the URL seems low value to me, since it requires that users understand that a search engine is actually represented as an xml file, or to make a value judgement on what has been repeatedly demonstrated as a spoofable entity (using idl or homographs or whatever). So I'm not even sure I see how this helps from a security perspective.

We are talking about adding search engines. This should be easy. Click - yes, that's what I wanted - click. Let's not throw things in the user's face unless they really want to.

If we can come up with some heuristics as to why search engines shouldn't be trusted and warn users at that time, that would be fine. Otherwise, I don't see this extra information as anything other than clutter.

Comment 32

[Pam Greene]

(In reply to comment #31)
> Adding the URL seems low value to me, since it requires that users
> understand that a search engine is actually represented as an xml file, or to
> make a value judgement on what has been repeatedly demonstrated as a spoofable
> entity (using idl or homographs or whatever). So I'm not even sure I see how
> this helps from a security perspective.

Well, without the URL I can't tell anything at all about what I'm installing other than the site-supplied title.  Would it be any better to trim the URL to the hostname rather than including the whole thing?

Comment 33

[Pam Greene]

Attachment: Patch with trimmed URL

This is no longer a work in progress.  This patch is ready for checkin whenever it passes reviews.  I'll be around to change the UI intermittently over the weekend; after that I guess we'll just have to take the hit if the strings need adjustment.

Comment 34

[Jesse Ruderman]

(In reply to comment #31)
> (In reply to comment #25)
> > Beltzner: Is the UI still acceptable?
> 
> IMO, no. Adding the URL seems low value to me, since it requires that users
> understand that a search engine is actually represented as an xml file, or to
> make a value judgement on what has been repeatedly demonstrated as a spoofable
> entity (using idl or homographs or whatever). So I'm not even sure I see how
> this helps from a security perspective.

Eh?  We're putting a lot of effort into making hostnames non-spoofable, including IDN stuff.

Comment 35

[Pam Greene]

Attachment: Screenshot, tentative title known

I'm no expert, but I like this one.  I think it strikes a happy medium between informative and geeky.  People know what providers and websites are, right?  Internet providers if not search providers as such, and at least with a hostname listed you can tell if the thing you're adding comes from somewhere wildly different from what it claims.

Comment 36

[Pam Greene]

Attachment: Screenshot, no title

Comment 37

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 225951 [details] [diff] [review]
Patch with trimmed URL

>Index: components/search/nsIBrowserSearchService.idl

>- *   Ben Goodger <[e-mail]> (Original author)
>- *   Gavin Sharp <[e-mail]>
>+ *   Ben Goodger   <[e-mail]> (Original author)
>+ *   Gavin Sharp   <[e-mail]>
>+ *   Pamela Greene <[e-mail]>

Probably best to leave the whitespace as is, we don't want to have to re-indent if someone with a longer name comes along :)

>Index: components/search/nsSearchService.js

>+  get uri() {
>+    return this._uri;
>+  },

No need for this, since this property isn't exposed. Just use _uri directly.

>   get currentEngine() {
...
>   },
>+  
>   set currentEngine(val) {

I was trying to keep getters/setters grouped together, I guess this doesn't really matter.

>-
>+  

ick, don't /add/ trailing whitespace! :)

>Index: components/sidebar/src/nsSidebar.js

>+  // With tentative title: Add "Somebody's Search" to the list...
>+  // With no title:        Add a new search engine to the list...
>+  var engineName;
>+  if (suggestedTitle) {
>+    engineName = stringBundle.formatStringFromName("addEngineQuotedEngineName",
>+                                                   [suggestedTitle], 1);
>+  }
>+  else {
>+    engineName = stringBundle.GetStringFromName("addEngineDefaultEngineName");
>+  }

I'd normally say remove these brackets, but I guess this file is so far away from following the browser/toolkit conventions that I should probably give up...

>+  // Display only the hostname portion of the URL.
>+  var uri = Components.classes["@mozilla.org/network/standard-url;1"]
>+                      .createInstance(Components.interfaces.nsIURI);
>+  uri.spec = engineURL;

nsIURIs should never be created by contract ID, instead use nsIIOService's newURI, which will know which protocol handler to use.

>-  if (!this.confirmSearchEngine(engineURL, suggestedTitle))
>+	var confirmation = this.confirmSearchEngine(engineURL, suggestedTitle);

eek, tab! :)

Comment 38

[Pam Greene]

Attachment: Patch addressing Gavin's comments

(In reply to comment #37)

All done, except:

> >+  get uri() {
> >+    return this._uri;
> >+  },
> 
> No need for this, since this property isn't exposed. Just use _uri directly.

It's used in SearchService's observer, SRCH_SVC_observe().  That's exposed -- or am I misunderstanding?

> I'd normally say remove these brackets, but I guess this file is so far away
> from following the browser/toolkit conventions that I should probably give
> up...

And just the other day, Darin wrote in a review of mine, "nit: favor brackets when coding an else block".  (Although that was in netwerk, not browser.)  Is there a style page I've missed?  I'd start one on the wiki, if only I knew which of the sometimes contradictory advice I've been given to put on it.

Comment 39

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #38)
> > No need for this, since this property isn't exposed. Just use _uri directly.
> 
> It's used in SearchService's observer, SRCH_SVC_observe().  That's exposed --
> or am I misunderstanding?

Ah, I see... it's still an implementation detail, though, not something to be accessed outside of nsSearchService.js (i.e. through xpcom).

> And just the other day, Darin wrote in a review of mine, "nit: favor brackets
> when coding an else block".  (Although that was in netwerk, not browser.)  Is
> there a style page I've missed?  I'd start one on the wiki, if only I knew
> which of the sometimes contradictory advice I've been given to put on it.

Every mozilla module can have different guidelines... Netwerk (Darin :) seems to favor braces, but browser/toolkit convention has been to omit them when possible. Doesn't help that some toolkit/browser peers that have differing opinions in certain cases :)

I've started a draft of style guidelines at http://developer.mozilla.org/en/docs/User:GavinSharp/JS_Style_Guidelines , borrowing heavily from http://wiki.mozilla.org/JavaScript:SpiderMonkey:Coding_Style and http://neil.rashbrook.org/JS.htm . That's my understanding of browser/toolkit style conventions, I'm going to work on that a bit and post to dev.apps.firefox for comments. Feedback welcome on the discussion page :)

Comment 40

[Pam Greene]

(In reply to comment #39)
> > > No need for this, since this property isn't exposed. Just use _uri directly.
> > 
> > It's used in SearchService's observer, SRCH_SVC_observe().  That's exposed --
> > or am I misunderstanding?
> 
> Ah, I see... it's still an implementation detail, though, not something to be
> accessed outside of nsSearchService.js (i.e. through xpcom).

Right.  So what does that mean for the method and style?  Does the class not need a getter for _uri if it's only used by another class in the same file?

Comment 41

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #32)
> (In reply to comment #31)
> Well, without the URL I can't tell anything at all about what I'm installing
> other than the site-supplied title.  Would it be any better to trim the URL to
> the hostname rather than including the whole thing?

Better, maybe, but better still would be to only issue a warning when we feel like there's something unexpected happening. If a user goes to www.foosite.com and clicks a link called "install google search engine" and then sees this dialog, it'll say:

  "Add Google to the list of engines available in the search bar?

   Provider: www.foosite.com"

There's no extra information there. Also, the majority of search plugins will likely be added from addons.mozilla.org. That "provider" will never match the name of the search plugin, so it's not even like we can expect users to expect that the engine name and provider name should have some root commonality.

If we were to only show this information when a) the engine refers to a search URL at a domain root which doesn't matter the source file and b) the source file isn't hosted at addons.mozilla.org, then I think we'd be getting somewhere, since we'd be applying a heuristic which gives us some confidence that the user is in some form of potential danger of being spoofed.

Since we're not doing this, the value to most users of the "Provider:" label still seems pretty low to me. 

Comment 42

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 225967 [details] [diff] [review]
Patch addressing Gavin's comments

In the interest of getting this landed, and because I do think it's a vast improvement over what we have, this gets ui-r+. I can track followup nits in followup bugs. Pam, as you check in, though, can you switch "Provider:" to "From:" which I think is much clearer language?

Comment 43

[Pam Greene]

(In reply to comment #41)
> If a user goes to www.foosite.com
> and clicks a link called "install google search engine" and then sees this
> dialog, it'll say:
> 
>   "Add Google to the list of engines available in the search bar?
> 
>    Provider: www.foosite.com"
> 
> There's no extra information there.

Only that it isn't saying "Provider: www.google.com", which is what I might expect.  Granted, I have to be a fairly sophisticated user to make that connection.

> If we were to only show this information when a) the engine refers to a search
> URL at a domain root which doesn't matter the source file and b) the source
> file isn't hosted at addons.mozilla.org, then I think we'd be getting
> somewhere, since we'd be applying a heuristic which gives us some confidence
> that the user is in some form of potential danger of being spoofed.

Case (a) isn't the situation I was thinking of as problematic.  If one site wants to promote another site's engine, that's fine.  What I really want to know is whether the human-readable name matches the engine I'm adding.  Unfortunately, I can't think of a way to verify that.

Comment 44

[Pam Greene]

(In reply to comment #42)
> Pam, as you check in, though, can you switch "Provider:" to
> "From:" which I think is much clearer language?

Will do.

Comment 45

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #43)
> Only that it isn't saying "Provider: www.google.com", which is what I might
> expect.  Granted, I have to be a fairly sophisticated user to make that
> connection.

But you've gone and browsed to www.foosite.com, which is where you clicked the link for the google search engine, so would it really be surprising that www.foosite.com is listed as where the search engine is coming from?

Also, you state:

> Case (a) isn't the situation I was thinking of as problematic.  If one site
> wants to promote another site's engine, that's fine.  What I really want to

That would be precisely the mismatch you were saying you'd be looking out for above. That means that not only do you have to notice the mismatch, but you'd also need to understand when that mismatch should be a problem vs. when it should not. If I was the type of user who understood all of that, I would also not be the type of user to be installing a spoofed search engine.

I guess what I'm saying is:

 - what are the spoofing cases we're really worried about?
 - how can we make those cases clear to the user?

I don't believe that adding the "From: www.foosite.com" will add a lot of clarity for the common user, but then again, I don't think the risks are all that bad, either :)

Comment 46

[Pam Greene]

(In reply to comment #45)
> > Case (a) isn't the situation I was thinking of as problematic.  If one site
> > wants to promote another site's engine, that's fine.  What I really want to
> 
> That would be precisely the mismatch you were saying you'd be looking out for
> above.

Sorry, I must have been unclear.  What I'd be looking out for is a mismatch between the engine's name and its provider.  So if I click a link at foosite.com and see
  
  Add "Google" to the list?
  From: www.google.com

I'll be more confident than if I see

  Add "Google" to the list?
  From: www.foosite.com

I might still install it, but at least I'd have a chance to be a little suspicious.  And even more so if what I clicked was a link in some blog comment on a different side entirely.

> If I was the type of user who understood all of that, I would also
> not be the type of user to be installing a spoofed search engine.

Even if you were that type of user, how would you tell the engine was spoofed if all you had was the author-provided name?  You'd have to go looking through the site's JS, which would be icky at best.

To be clear, I completely agree that none of this is very effective.  I just think it's better than nothing.

Comment 47

[Jesse Ruderman]

www.google.com has several open redirects, so it isn't sufficient to show that www.google.com is the hostname used in the search plugin.  That's why I think it makes more sense to show where the search plugin came from.

Comment 48

[Pam Greene]

Well, if I can get a code review, I'll check this in as it stands and follow up later.  It's not too bad to change one or two strings in the next few days, and trivial to switch from hostname back to full URI pretty much anytime.

Comment 49

[Mike Connor [:mconnor]]

Comment on attachment 225967 [details] [diff] [review]
Patch addressing Gavin's comments

>+  // With tentative title: Add "Somebody's Search" to the list...
>+  // With no title:        Add a new search engine to the list...
>+  var engineName;
>+  if (suggestedTitle) {
>+    engineName = stringBundle.formatStringFromName("addEngineQuotedEngineName",
>+                                                   [suggestedTitle], 1);
>+  }
>+  else {
>+    engineName = stringBundle.GetStringFromName("addEngineDefaultEngineName");
>+  }

nit: unnecessary braces

>+  // Display only the hostname portion of the URL.
>+  var ios = Components.classes["@mozilla.org/network/io-service;1"]
>+                      .getService(Components.interfaces.nsIIOService);
>+  var uri = ios.newURI(engineURL, null, null);

newURI can throw (i.e. if someone got the args wrong), what happens in that case?

So, I'm ok with this for now, though I'm waffling on the spoofing implications

Comment 50

[Pam Greene]

Attachment: Patch as checked into trunk

Changes "Provider" to "From"; removes braces some consider unnecessary :); adds try{} around newURI; patches trunk properly.

Comment 51

[Pam Greene]

fixed-1.8-branch, fixed-on-trunk