Closed
Bug 553459
Opened 15 years ago
Closed 14 years ago
Deal with saved POST data in functions loading entries e.g. from places
Categories
(SeaMonkey :: UI Design, defect)
SeaMonkey
UI Design
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.1b3
People
(Reporter: kairo, Assigned: philip.chee)
References
(Depends on 1 open bug)
Details
Attachments
(5 files, 8 obsolete files)
(deleted),
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•15 years ago
|
||
Also, some getShortcutOrURI calls will need investigation to retrieve the postData, see http://mxr.mozilla.org/comm-central/search?string=getShortcutOrURI
Reporter | ||
Updated•15 years ago
|
No longer blocks: SMPlacesBMarks
Depends on: SMPlacesBMarks
Reporter | ||
Updated•15 years ago
|
OS: Linux → All
Hardware: x86 → All
Version: SeaMonkey 2.0 Branch → unspecified
Reporter | ||
Comment 2•14 years ago
|
||
I think some recent work of Misak or Ratty has done some of the postdata things, so bringing them into the loop here.
Assignee | ||
Comment 3•14 years ago
|
||
> 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.
Reporter | ||
Comment 4•14 years ago
|
||
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.
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•14 years ago
|
||
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.
Assignee | ||
Comment 6•14 years ago
|
||
> - 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.
Attachment #489507 -
Flags: review?(neil)
Comment 7•14 years ago
|
||
(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 10•14 years ago
|
||
Try evaluating this in the Error Console: top.Services.search.getEngineByName('Google').alias = 'g';
Reporter | ||
Comment 11•14 years ago
|
||
(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.
Assignee | ||
Comment 12•14 years ago
|
||
> 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•14 years ago
|
||
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.
Assignee | ||
Comment 14•14 years ago
|
||
> neil@parkwaycc.co.uk 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.
Attachment #489507 -
Attachment is obsolete: true
Attachment #492354 -
Flags: review?(neil)
Attachment #489507 -
Flags: review?(neil)
Comment 15•14 years ago
|
||
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•14 years ago
|
||
(In reply to comment #14) > I think I can just pass all the arguments to loadURI() and handle the > legacy flags there. Neat :-)
Assignee | ||
Comment 17•14 years ago
|
||
>>+ 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.
Attachment #492354 -
Attachment is obsolete: true
Attachment #492599 -
Flags: review?(neil)
Attachment #492354 -
Flags: review?(neil)
Assignee | ||
Comment 18•14 years ago
|
||
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.
Attachment #492599 -
Attachment is obsolete: true
Attachment #493211 -
Flags: review?(neil)
Attachment #492599 -
Flags: review?(neil)
Updated•14 years ago
|
Attachment #493211 -
Attachment is patch: true
Attachment #493211 -
Attachment mime type: application/octet-stream → text/plain
Comment 19•14 years ago
|
||
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.
Assignee | ||
Comment 20•14 years ago
|
||
> neil@parkwaycc.co.uk 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.
Attachment #493211 -
Attachment is obsolete: true
Attachment #495250 -
Flags: review?(neil)
Attachment #493211 -
Flags: review?(neil)
Comment 21•14 years ago
|
||
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.
Assignee | ||
Comment 22•14 years ago
|
||
> 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?
Attachment #495250 -
Attachment is obsolete: true
Attachment #495499 -
Flags: review?(neil)
Attachment #495250 -
Flags: review?(neil)
Comment 23•14 years ago
|
||
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.)
Attachment #495499 -
Flags: review?(neil) → review+
Assignee | ||
Comment 24•14 years ago
|
||
Pushed to comm-central http://hg.mozilla.org/comm-central/rev/639c664ceb8f
Assignee | ||
Updated•14 years ago
|
Attachment #495518 -
Attachment description: Patch v1.0 Part 1: loadURI() and friends. (11.55 KB, patch)
2010-11-10 08:33 PST, Philip Chee no flags Details | Diff
Patch v1.1 Part 1: loadURI() As checked in. → Patch v1.1 Part 1: loadURI() As checked in.
Assignee | ||
Comment 25•14 years ago
|
||
>>+ 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.
Assignee | ||
Updated•14 years ago
|
Attachment #495518 -
Attachment description: Patch v1.1 Part 1: loadURI() As checked in. → Patch v1.4a Part 1: loadURI() As checked in.
Assignee | ||
Comment 26•14 years ago
|
||
+++ 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.
Attachment #502473 -
Flags: review?(neil)
Comment 27•14 years ago
|
||
(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•14 years ago
|
||
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
Attachment #502473 -
Flags: review?(neil) → review-
Assignee | ||
Comment 29•14 years ago
|
||
> neil@parkwaycc.co.uk 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.
Attachment #502473 -
Attachment is obsolete: true
Attachment #507861 -
Flags: review?(neil)
Comment 31•14 years ago
|
||
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)?
Assignee | ||
Updated•14 years ago
|
Attachment #507861 -
Flags: review?(neil)
Assignee | ||
Comment 32•14 years ago
|
||
> neil@parkwaycc.co.uk 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.
Attachment #507861 -
Attachment is obsolete: true
Attachment #508114 -
Flags: review?(neil)
Updated•14 years ago
|
Attachment #508114 -
Flags: review?(neil) → review+
Assignee | ||
Comment 33•14 years ago
|
||
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
Attachment #508114 -
Attachment description: Patch Bv1.1 (openNewTabWith/openNewWindowWith) → [Checked in Comment 33] Patch Bv1.1 (openNewTabWith/openNewWindowWith)
Comment 34•14 years ago
|
||
Comment on attachment 507862 [details] [diff] [review] Patch Cv1.0 goButtonObserver Hmm... don't the other paths do the security check before getShortcutOrURI?
Assignee | ||
Comment 35•14 years ago
|
||
> 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•14 years ago
|
||
(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.
Assignee | ||
Comment 37•14 years ago
|
||
So Neil, do you want me to move the security check before getShortcutOrURI() or what?
Assignee | ||
Comment 39•14 years ago
|
||
>> So Neil, do you want me to move the security check before getShortcutOrURI() or >> what? > > neil@parkwaycc.co.uk 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.
Assignee | ||
Comment 40•14 years ago
|
||
>> 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.
Attachment #507862 -
Attachment is obsolete: true
Attachment #516897 -
Flags: review?(neil)
Attachment #507862 -
Flags: review?(neil)
Comment 41•14 years ago
|
||
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.
Attachment #516897 -
Flags: review?(neil) → review+
Assignee | ||
Comment 42•14 years ago
|
||
Pushed to comm-central. http://hg.mozilla.org/comm-central/rev/18feac933586 > neil@parkwaycc.co.uk 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.
Assignee | ||
Comment 43•14 years ago
|
||
Closing as FIXED! Additional postData patches to be filed in new bugs please!
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 44•14 years ago
|
||
(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•14 years ago
|
||
(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...
Assignee | ||
Comment 46•14 years ago
|
||
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
Updated•14 years ago
|
Attachment #495518 -
Attachment description: Patch v1.4a Part 1: loadURI() As checked in. → [Checked in Comment 24]Patch v1.4a Part 1: loadURI() As checked in
Updated•14 years ago
|
Attachment #495518 -
Attachment description: [Checked in Comment 24]Patch v1.4a Part 1: loadURI() As checked in → [Checked in Comment 24] Patch v1.4a Part 1: loadURI() As checked in
Updated•14 years ago
|
Target Milestone: --- → seamonkey2.1b3
Version: unspecified → Trunk
You need to log in
before you can comment on or make changes to this bug.
Description
•