553459 - Deal with saved POST data in functions loading entries e.g. from places

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Robert Kaiser]

Firefox can load URIs with saved POST data from e.g. places, some examples are here:
http://mxr.mozilla.org/comm-central/source/mozilla/browser/base/content/browser.js#909
http://mxr.mozilla.org/comm-central/source/mozilla/browser/base/content/browser.js#1904

In bug 498596, I'm porting some calls to hand us the POST data, but we need to do further work across the board to actually call up entries with it.

Comment 1

[Robert Kaiser]

Also, some getShortcutOrURI calls will need investigation to retrieve the postData, see http://mxr.mozilla.org/comm-central/search?string=getShortcutOrURI

Comment 2

[Robert Kaiser]

I think some recent work of Misak or Ratty has done some of the postdata things, so bringing them into the loop here.

Comment 3

[Philip Chee]

> http://mxr.mozilla.org/comm-central/source/mozilla/browser/base/content/browser.js#909
> http://mxr.mozilla.org/comm-central/source/mozilla/browser/base/content/browser.js#1904
These seem to be pointed at random locations in browser.js at the moment, could you use specific changesets when specifying MXR urls? Thanks.

Comment 4

[Robert Kaiser]

For one thing, those were just examples, for the other MXR can't do specific revisions and hg file display is quite slow, doesn't come up in MXR search results and is not as pretty as MXR, that's why I used those URLs and will do so again in the future.

Based on a hg pushlog search for the date when I posted this, you can of course look at http://hg.mozilla.org/mozilla-central/file/cc33a4008a78/browser/base/content/browser.js#l909 and http://hg.mozilla.org/mozilla-central/file/cc33a4008a78/browser/base/content/browser.js#l1897 (the latter is adjusted to point at the beginning of the function) instead, just for some examples. A search for "postData" probably would do a better job, actually.

Comment 5

[Philip Chee]

I'm going to break this up into two or more parts to make things simpler.

1. Fix-up loadURI() and all dependencies.
2. Fix-up openTabWith()/openWindowWith() and dependencies.
3. Anything else.

Comment 6

[Philip Chee]

Attachment: Patch v1.0 Part 1: loadURI() and friends.

> -      gBrowser.loadURIWithFlags(submission.uri.spec, nsIWebNavigation.LOAD_FLAGS_NONE,
> -                                null, null, submission.postData);
> +      loadURI(submission.uri.spec, null, submission.postData, false);

Krang changed loadURI() to gBrowser.loadURIWithFlags(). Now that we support postData reverting to loadURI().

> @@ -1600,17 +1609,22 @@ function getShortcutOrURI(aURL, aPostDat
> -  // XXX: In the future, ask the search service if it knows that keyword
> +  var engine = Services.search.getEngineByAlias(keyword);
> +  if (engine) {
> +    var submission = engine.getSubmission(param);
> +    aPostDataRef.value = submission.postData;
> +    return submission.uri.spec;
> +  }

Not sure how to test this.

> @@ -1229,34 +1229,30 @@ function openUILinkIn(url, where, aAllow
> -    w.loadURI(url, aPostData, flags);
> +    w.loadURI(url, aReferrerURI, aPostData, aAllowThirdPartyFixup);

The order of parameters to loadURI() was wrong anyway you looked at it. Not sure how this could have worked at all.

Comment 7

[[e-mail]]

(In reply to comment #6)
> > +  var engine = Services.search.getEngineByAlias(keyword);
> > +  if (engine) {
> > +    var submission = engine.getSubmission(param);
> > +    aPostDataRef.value = submission.postData;
> > +    return submission.uri.spec;
> > +  }
> Not sure how to test this.
Just enter "google foo" or "yahoo foo" as if it was a bookmark keyword?
[N.B. Not sure what the actual aliases are for the engines.]

> > @@ -1229,34 +1229,30 @@ function openUILinkIn(url, where, aAllow
> > -    w.loadURI(url, aPostData, flags);
> > +    w.loadURI(url, aReferrerURI, aPostData, aAllowThirdPartyFixup);
> The order of parameters to loadURI() was wrong anyway you looked at it. Not
> sure how this could have worked at all.
Well, I doubt anyone passed in any post data, and if we're lucky they didn't want to allow third party fixup either, in which case they were probably passing in undefined anyway.

Comment 8

[[e-mail]]

Turns out they don't seem to have any by default...

Comment 9

[[e-mail]]

Ah, it seems that Firefox has an alias editor for search engines.

Comment 10

[[e-mail]]

Try evaluating this in the Error Console:

top.Services.search.getEngineByName('Google').alias = 'g';

Comment 11

[Robert Kaiser]

(In reply to comment #9)
> Ah, it seems that Firefox has an alias editor for search engines.

BTW, a port for that should be up in the WIP search bar patch, if someone wants to pick that up.

Comment 12

[Philip Chee]

> Try evaluating this in the Error Console:
> top.Services.search.getEngineByName('Google').alias = 'g';

Services.search.getEngineByName('Google').alias = 'g';
-> "g"

var postData = {};
[getShortcutOrURI("g wikipedia seamonkey", postData), postData.value];

-> ["http://www.google.com/search?q=wikipedia+seamonkey&start=0&start=0&ie=utf-8&oe=utf-8", null]

Typing "g wikipedia seamonkey" into the urlbar works. And I get a google search results page e.g.
http://de.wikipedia.org/wiki/SeaMonkey
"SeaMonkey ist eine aus der Mozilla Application Suite hervorgegangene, freie Programmsammlung, die aus Webbrowser, E-Mail-Programm und weiteren Werkzeugen besteht."

Comment 13

[[e-mail]]

Comment on attachment 489507 [details] [diff] [review]
Patch v1.0 Part 1: loadURI() and friends.

>+/* window.arguments[0]: URL(s) to load
>+                        (string, with one or more URLs separated by \n)
>+ *                 [1]: character set (string)
>+ *                 [2]: referrer (nsIURI)
>+ *                 [3]: postData (nsIInputStream)
Argument 3 used to be loadFlags. Should we put in some fallback code in case someone tries to pass in flags? Or should we put postData 4th?

>-function loadURI(uri, referrer, flags)
>+function loadURI(uri, referrer, postData, allowThirdPartyFixup)
Similarly here.

>+    if (postData === undefined)
>+      postData = null;
Unnecessary; XPConnect will convert undefined to null for you.

Comment 14

[Philip Chee]

Attachment: Patch v1.1 Part 1: loadURI() Handle legacy load flags.

> [e-mail]      2010-11-21 12:58:06 PST
> 
> >+ *                 [3]: postData (nsIInputStream)
> Argument 3 used to be loadFlags. Should we put in some fallback code in case
> someone tries to pass in flags? Or should we put postData 4th?
I would prefer to put in fallback code because not matching the order in Firefox makes it harder to port Firefox extensions.

Hmm. I think I can just pass all the arguments to loadURI() and handle the legacy flags there.

> >-function loadURI(uri, referrer, flags)
> >+function loadURI(uri, referrer, postData, allowThirdPartyFixup)
> Similarly here.
Added legacy load flags handling code.

> >+    if (postData === undefined)
> >+      postData = null;
> Unnecessary; XPConnect will convert undefined to null for you.
Fixed.

Comment 15

[[e-mail]]

Comment on attachment 492354 [details] [diff] [review]
Patch v1.1 Part 1: loadURI() Handle legacy load flags.

>+    else if (postData && number == typeof postData) {
Don't you mean typeof postData == "number" with quotes?
(You don't need to null-check postData because typeof null != "number")

Comment 16

[[e-mail]]

(In reply to comment #14)
> I think I can just pass all the arguments to loadURI() and handle the
> legacy flags there.
Neat :-)

Comment 17

[Philip Chee]

Attachment: Patch v1.1a Part 1: loadURI() Handle legacy load flags (properly).

>>+    else if (postData && number == typeof postData) {
> Don't you mean typeof postData == "number" with quotes?
> (You don't need to null-check postData because typeof null != "number")

Sigh. The try catch block was hiding the actual error.
Fixed.

To check. Right click on a link and open in a new window.

Comment 18

[Philip Chee]

Attachment: Patch v1.2 Part 1: loadURI() fold BrowserLoadURL() into handleURLBarCommand()

While looking into the go-button fixes in URLBar FixUps (Bug 613199), I noticed that everything that calls BrowserLoadURL() calls handleURLBarCommand() first. So I folded BrowserLoadURL() into handleURLBarCommand() and pointed all the consumers at the latter.


>  function handleURLBarCommand(aUserAction, aTriggeringEvent)
>  {
> +  var postData = {};
> +  var url = getShortcutOrURI(gURLBar.value.trim(), postData);
> +
> +  gURLBar.value = url;
> +  gBrowser.userTypedValue = url;
> +
>    try {
>      addToUrlbarHistory(gURLBar.value);


I moved getShortcutOrURI() before addToUrlbarHistory() as Firefox does that. Not sure if this is correct.

Comment 19

[[e-mail]]

Comment on attachment 493211 [details] [diff] [review]
Patch v1.2 Part 1: loadURI() fold BrowserLoadURL() into handleURLBarCommand()

>+    else if ("number" == typeof postData) {
Nit: I prefer typeof postData == "number"

>-function BrowserLoadURL(aTriggeringEvent)
Please could you move handleURLBarCommand here?
[destroys less blame/makes patch easier to read etc.]

> function handleURLBarCommand(aUserAction, aTriggeringEvent)
> {
>+  var postData = {};
>+  var url = getShortcutOrURI(gURLBar.value.trim(), postData);
>+
>+  gURLBar.value = url;
>+  gBrowser.userTypedValue = url;
>+
>   try {
>     addToUrlbarHistory(gURLBar.value);
I think the value we want to add to URLbar history is the one that the user actually typed, not the bookmark keyword value or search engine query.

Comment 20

[Philip Chee]

Attachment: Patch v1.3 Part 1: loadURI() fix nits.

> [e-mail]      2010-12-03 16:17:37 PST
> 
>>+    else if ("number" == typeof postData) {
> Nit: I prefer typeof postData == "number"
Fixed.

>>-function BrowserLoadURL(aTriggeringEvent)
> Please could you move handleURLBarCommand here?
> [destroys less blame/makes patch easier to read etc.]
Fixed.

>> function handleURLBarCommand(aUserAction, aTriggeringEvent)
>> {
>>+  var postData = {};
>>+  var url = getShortcutOrURI(gURLBar.value.trim(), postData);
>>+
>>+  gURLBar.value = url;
>>+  gBrowser.userTypedValue = url;
>>+
>>   try {
>>     addToUrlbarHistory(gURLBar.value);
> I think the value we want to add to URLbar history is the one that the user
> actually typed, not the bookmark keyword value or search engine query.
Fixed.

Comment 21

[[e-mail]]

Comment on attachment 495250 [details] [diff] [review]
Patch v1.3 Part 1: loadURI() fix nits.

>+  
Nit: trailing spaces

>+  var postData = {};
>+  var url = getShortcutOrURI(gURLBar.value.trim(), postData);
OK, so this is a change in behaviour; before, if you used a bookmark keyword for a view source URL then it would open in the requested tab/window but now it opens in a view source window. I can't find the equivalent code for Firefox but then perhaps it doesn't have it?

>+  gURLBar.value = url;
>+  gBrowser.userTypedValue = url;
And this is also a change in behaviour; before, if you used a bookmark keyword for a javascript URL (bookmarklet) then it would leave the keyword in the URLbar but now it leaves the javascript URL in the URLbar. This change was sneaked in without explanation in bug 454109.

Comment 22

[Philip Chee]

Attachment: Patch v1.4 Part 1: loadURI() flyspecking

> Comment on attachment 495250 [details] [diff] [review]
>>+  
> Nit: trailing spaces
Fixed.

>>+  gURLBar.value = url;
>>+  gBrowser.userTypedValue = url;
> And this is also a change in behaviour; before, if you used a bookmark keyword
> for a javascript URL (bookmarklet) then it would leave the keyword in the
> URLbar but now it leaves the javascript URL in the URLbar. This change was
> sneaked in without explanation in bug 454109.
Fixed.

>>+  var postData = {};
>>+  var url = getShortcutOrURI(gURLBar.value.trim(), postData);
> OK, so this is a change in behaviour; before, if you used a bookmark keyword
> for a view source URL then it would open in the requested tab/window but now it
> opens in a view source window.
Fixed.

> I can't find the equivalent code for Firefox but
> then perhaps it doesn't have it?

I dug through CVS blame (Bleah!)

Originally both the Suite and Firefox did the same:

view-source: urls would load in the current tab.
view-source: links would follow the tabbrowser/link behaviour preferences.

bz changed this in:
Bug 78619 View-source: pages have a view source option if opened by hyperlink

> OK.  The "problem" here is that the view-source: URL is loaded in the normal
> navigator window with all its menus and context menus and whatnot, right?
> 
> The options I see are:
> 1)  Kick off the viewsource window for view-source: urls in navigator (using JS)
> 2)  Disable the C-u shortcut, View Source context menu option, and View Source
>     menuitem when the url is a view-source: one
> 
> Neither of these is parser.  Over to XP Apps.
> 
> #1 is probably simpler to do, but #2 is the one that seems like the more correct
> solution....

bz chose #1

My take on this is that |BrowserViewSourceOfDocument(aDocument)| should have been fixed to strip off any /^view-source:/ scheme before passing the URL to |gViewSourceUtils.viewSource(webNav.currentURI.spec, pageCookie, aDocument)|

I'm not sure what to pass for the pageCookie in this case.

Should I file a bug to revert our view-source: behaviour to match Firefox?

Comment 23

[[e-mail]]

Comment on attachment 495499 [details] [diff] [review]
Patch v1.4 Part 1: loadURI() flyspecking

>   // Remove leading and trailing spaces first
>   var url = gURLBar.value.trim();
>+  try {
>+    addToUrlbarHistory(gURLBar.value);
You might want to pass url rather than fetching gURLBar.value again.
(addToUrlbarHistory calls trim() so it makes no difference.)

>+  } catch (ex) {
>+    // Things may go wrong when adding url to session history,
We should fix this comment to say location bar history.
(I meant to mention it before but it somehow slipped my mind.)

Comment 24

[Philip Chee]

Attachment: [Checked in Comment 24] Patch v1.4a Part 1: loadURI() As checked in

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/639c664ceb8f

Comment 25

[Philip Chee]

>>+    addToUrlbarHistory(gURLBar.value);
> You might want to pass url rather than fetching gURLBar.value again.
> (addToUrlbarHistory calls trim() so it makes no difference.)

>>+    // Things may go wrong when adding url to session history,
> We should fix this comment to say location bar history.
> (I meant to mention it before but it somehow slipped my mind.)

All Fixed on checkin.

Comment 26

[Philip Chee]

Attachment: Patch Bv1.0 (openNewTabWith/openNewWindowWith)

+++ b/suite/browser/navigatorDD.js
   onDrop: function (aEvent, aXferData, aDragSession)
     {
       var xferData = aXferData.data.split("\n");
-      var uri = xferData[0] ? xferData[0] : xferData[1];
-      if (uri)
-        {
-          // Perform a security check before loading the URI
-          nsDragAndDrop.dragDropSecurityCheck(aEvent, aDragSession, uri);
+      var draggedText = xferData[0] || xferData[1];
+      var postData = {};
+      var url = getShortcutOrURI(draggedText, postData);
+      try {
+        nsDragAndDrop.dragDropSecurityCheck(aEvent, aDragSession, url);
 
-          loadURI(uri);
-        }
+        const nsIScriptSecMan = Components.interfaces.nsIScriptSecurityManager;
+        urlSecurityCheck(url, gBrowser.mCurrentBrowser.contentPrincipal,
+                         nsIScriptSecMan.DISALLOW_SCRIPT_OR_DATA);
+        loadURI(url, null, postData.value, true);

Missed loadURI() from first round. Also ported part of Bug 291651 (Address bar accepts dragged javascript urls) which we might not want. Neil?
The signature of urlSecurityCheck() changed since the Firefox patch and the current code doesn't contain the go-button anymore. Is |gBrowser.mCurrentBrowser.contentPrincipal| correct?

+      } catch (ex) {
+        Components.classes["@mozilla.org/consoleservice;1"]
+                  .getService(Components.interfaces.nsIConsoleService)
+                  .logStringMessage(ex);
+      }

I put this in for debugging because I wasn't sure what to put for the principal.

+++ b/suite/common/nsContextMenu.js
   openLink: function() {
     // Determine linked-to URL.
-    openNewWindowWith(this.linkURL, this.target.ownerDocument);
+    return openNewWindowWith(this.linkURL, this.target.ownerDocument);

Firefox Bug 423833 added returns to make writing tests easier.

   openFrame: function() {
-    openNewWindowWith( this.target.ownerDocument.location.href );
+    return openNewWindowWith(this.target.ownerDocument.location.href,
+                             this.target.ownerDocument);

Firefox wants to send a referrer.

+++ b/suite/common/utilityOverlay.js
-function openNewWindowWith(aURL, aDoc)
+function openNewWindowWith(aURL, aDoc, aPostData, aAllowThirdPartyFixup,
+                           aReferrer)

As far as I can tell the only reason for the existence of the superfluous aReferrer parameter is so that Florian Quèze in Bug 376189 can bypass the security check on the referrer.

Comment 27

[[e-mail]]

(In reply to comment #26)
> As far as I can tell the only reason for the existence of the superfluous
> aReferrer parameter is so that Florian Quèze in Bug 376189 can bypass the
> security check on the referrer.
To be fair, I don't think he can guarantee the existence of a document.

Comment 28

[[e-mail]]

Comment on attachment 502473 [details] [diff] [review]
Patch Bv1.0 (openNewTabWith/openNewWindowWith)

Sorry for not getting to this earlier.

>       var xferData = aXferData.data.split("\n");
This isn't working for whatever reason, but it means I can't check this part of the patch. OK to leave it for a followup?

>+function openNewTabWith(aURL, aDoc, aEvent, aPostData,
>+                        aAllowThirdPartyFixup, aReferrer)
Firefox uses aPostData, aEvent

Comment 29

[Philip Chee]

Attachment: Patch Bv1.1 (openNewTabWith/openNewWindowWith)

> [e-mail]      2011-01-20 08:30:13 PST
> 
> Comment on attachment 502473 [details] [diff] [review]
> Patch Bv1.0 (openNewTabWith/openNewWindowWith)
> 
> Sorry for not getting to this earlier.
> 
>>       var xferData = aXferData.data.split("\n");
> This isn't working for whatever reason, but it means I can't check this part of
> the patch. OK to leave it for a followup?
Sure.

>>+function openNewTabWith(aURL, aDoc, aEvent, aPostData,
>>+                        aAllowThirdPartyFixup, aReferrer)
> Firefox uses aPostData, aEvent
Ooops. Fixed.

Comment 30

[Philip Chee]

Attachment: Patch Cv1.0 goButtonObserver

Comment 31

[[e-mail]]

Comment on attachment 507861 [details] [diff] [review]
Patch Bv1.1 (openNewTabWith/openNewWindowWith)

>-  if (pref) {
Heh.

>+  if (arguments.length > 2)
[The other checks will fail anyway, since they'd be undefined.]

>+    if (typeof aPostData == "boolean")
>+    {
>+      // Handle legacy boolean parameter.
>+      if (aPostData)
>+        loadInBackground = !loadInBackground;
Don't you need to set aPostData to null if it's a boolean (true or false)?

Comment 32

[Philip Chee]

Attachment: [Checked in Comment 33] Patch Bv1.1 (openNewTabWith/openNewWindowWith)

> [e-mail]      2011-01-28 08:12:10 PST

>>+  if (arguments.length > 2)
> [The other checks will fail anyway, since they'd be undefined.]
I thought I'd bullet proof it a bit, but I guess that's not necessary. Fixed.

>>+    if (typeof aPostData == "boolean")
>>+    {
>>+      // Handle legacy boolean parameter.
>>+      if (aPostData)
>>+        loadInBackground = !loadInBackground;
> Don't you need to set aPostData to null if it's a boolean (true or false)?
Oh, indeed. Fixed.

Comment 33

[Philip Chee]

Comment on attachment 508114 [details] [diff] [review]
[Checked in Comment 33] Patch Bv1.1 (openNewTabWith/openNewWindowWith)

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/e1c66e0e8ce3

Comment 34

[[e-mail]]

Comment on attachment 507862 [details] [diff] [review]
Patch Cv1.0 goButtonObserver

Hmm... don't the other paths do the security check before getShortcutOrURI?

Comment 35

[Philip Chee]

> Hmm... don't the other paths do the security check before getShortcutOrURI?

None of the other callers in Suite do a security check before getShortcutOrURI(). I also checked callers of the callers.

In Minefield:
Of the eight callers, only contentAreaClick() calls urlSecurityCheck() before getShortcutOrURI() but only in one of two code paths. And it only does a security check if the click comes from the web-panel sidebar.

In Firefox 3.5 and 3.6:
1. The newTabButtonObserver and the newWindowButtonObserver do a nsDragAndDrop.dragDropSecurityCheck *after* calling getShortcutOrURI.
2. contentAreaDNDObserver does a nsDragAndDrop.dragDropSecurityCheck before getShortcutOrURI. This is gone in Minefield.

In Firefox 3.5 and 3.6 the tabbrowser _onDrop handler did a nsDragAndDrop.dragDropSecurityCheck before getShortcutOrURI but this disappeared in Firefox 4.0. Also removed was this comment:
// XXXmano: temporary fix until dragDropSecurityCheck make the
// drag-session an optional paramter

I decline to dig through blame to trace all the intermediate states.

Comment 36

[[e-mail]]

(In reply to comment #35)
> In Firefox 3.5 and 3.6 the tabbrowser _onDrop handler did a
> nsDragAndDrop.dragDropSecurityCheck before getShortcutOrURI but this
> disappeared in Firefox 4.0.
I think ContentAreaDropListener::_validateURI does the check in the tabbrowser case these days.

Comment 37

[Philip Chee]

So Neil, do you want me to move the security check before getShortcutOrURI() or what?

Comment 38

[[e-mail]]

Yes please.

Comment 39

[Philip Chee]

>> So Neil, do you want me to move the security check before getShortcutOrURI() or
>> what?
> 
> [e-mail] 2011-03-03 05:08:08 PST
> 
> Yes please.

Problem when moving the security check before getShortcutOrURI()

Error: Load of Bug 291651 denied.


1. STR: Create a keyword search e.g.

Name: Bugzilla Keyword Search
Location: https://bugzilla.mozilla.org/show_bug.cgi?id=%s
Keyword: bug

2. Open a text file in a tab with the following text:

"Also ported part of Bug 291651 (Address bar accepts dragged javascript urls)"

3. Highlight "Bug 291651" and try to DnD it on the Go button.

Comment 40

[Philip Chee]

Attachment: Patch Cv1.1 goButtonObserver

>> So Neil, do you want me to move the security check before getShortcutOrURI() or
>> what?
> 
> Yes please.

> <NeilAway>	RattyAway: hmm, that check only works on actual uris
> <NeilAway>	RattyAway: ideally you would do something along the lines of contentAreaDropListener.js's _validateURI
Fixed.

Comment 41

[[e-mail]]

Comment on attachment 516897 [details] [diff] [review]
Patch Cv1.1 goButtonObserver

>+          urlSecurityCheck(uri, gBrowser.mCurrentBrowser.contentPrincipal,
>+                           nsIScriptSecMan.DISALLOW_SCRIPT_OR_DATA);
Never use mCurrentBrowser outside tabbrowser.xml! Best idea here is to use content.document.nodePrincipal in the first place. r=me with this fixed.

>+      } catch (ex) {
>+        Services.console.logStringMessage(ex);
>+      }
Don't quite see why you need to catch and log this.

Comment 42

[Philip Chee]

Attachment: [Checked in Comment 42] Patch Cv1.1a goButtonObserver As checked in.

Pushed to comm-central.
http://hg.mozilla.org/comm-central/rev/18feac933586

> [e-mail]      2011-03-04 08:32:57 PST
> 
>>+          urlSecurityCheck(uri, gBrowser.mCurrentBrowser.contentPrincipal,
>>+                           nsIScriptSecMan.DISALLOW_SCRIPT_OR_DATA);
> Never use mCurrentBrowser outside tabbrowser.xml! Best idea here is to use
> content.document.nodePrincipal in the first place. r=me with this fixed.
Fixed.

>>+      } catch (ex) {
>>+        Services.console.logStringMessage(ex);
>>+      }
> Don't quite see why you need to catch and log this.
Removed.

Note: dropping random objects from the Win7 desktop on to the go button tends to result in:

Error: aXferData.data.split is not a function
Source file: chrome://navigator/content/navigatorDD.js
Line: 178

But then that happens even without this patch.

Comment 43

[Philip Chee]

Closing as FIXED! Additional postData patches to be filed in new bugs please!

Comment 44

[Robert Kaiser]

(In reply to comment #42)
> Note: dropping random objects from the Win7 desktop on to the go button tends
> to result in:
> 
> Error: aXferData.data.split is not a function
> Source file: chrome://navigator/content/navigatorDD.js
> Line: 178
> 
> But then that happens even without this patch.

We have a lot of outdated drag and drop code lying around, I guess we're using all 3 generations of dnd, even though we should ideally only use the new HTML5 one. That surely belongs in other bugs, though.

Comment 45

[[e-mail]]

(In reply to comment #42)
>>>+      } catch (ex) {
>>>+        Services.console.logStringMessage(ex);
>>>+      }
>>Don't quite see why you need to catch and log this.
>Removed.
Well, you removed the logging, but I was hoping for the try/catch too...

Comment 46

[Philip Chee]

Bug 553459 postData goButtonObserver followup (Remove try/catch)

> Well, you removed the logging, but I was hoping for the try/catch too...
Sigh, fixed, pushed.

http://hg.mozilla.org/comm-central/rev/4087c9b318d3