Closed Bug 160106 Opened 18 years ago Closed 9 years ago

support referrer property in GetURL AppleScript command

Categories

(Camino Graveyard :: OS Integration, enhancement, P3)

All
macOS
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: stf, Assigned: alqahira)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [camino-2.1.1])

Attachments

(2 files, 1 obsolete file)

Would it be possible to have an extended GetURL, which might be called OpenURL
(like in Chimera 1.1a) with the same features AND [from string] (like in GetURL)
to be able to define the Referrer.
Because some sites are 'protected' and refuse direct access if the Referrer is
not the expected one.
If the [toWindow integer] is omitted, the file is simply saved, so files can be
downloaded very quickly without displaying them. Thanks !

Extract from Chimera AppleScript dictionnary:

OpenURL: Opens a URL. Allows for more options than GetURL event
OpenURL  string  -- URL
  [to  'FSS ']  -- file destination
  [toWindow  integer]  -- window ID
  [from  string]  -- Referrer, to be sent with the HTTP request
  [flags  integer]  -- Binary: any combination of 1, 2 and 4 is allowed: 1 and 2
mean force reload the document. 4 is ignored
  [post data  string]  -- Form posting data
  [post type  string]  -- MIME type of the posting data. Defaults to
application/x-www-form-urlencoded
  [progressApp  'psn ']  -- Application that will display progress
Result:   integer  -- ID of the loading window
Blocks: 125419
Wow, Deja Vu, there was an OpenURL event in the Spyglass AE suite.  I think it's
still in the AE disctionary for Netscape 4.x.  Might I suggest a different name
for folks that have a memory as old as mine?  ExtendedGetURL maybe?
Mine
Assignee: saari → sfraser
Summary: [RFE] OpenURL = AppleScript extended GetURL → OpenURL = AppleScript extended GetURL
We used to have the Spyglass OpenURL (see bug 197643).  This bug is also asking
for the ability to set a referrer property.

old field values:
summary: OpenURL = AppleScript extended GetURL
depends on: blank

The reporter might also google for 'pornzilla' to accomplish some of this with
bookmarklets.
Depends on: 197643
Summary: OpenURL = AppleScript extended GetURL → support referrer property in OpenURL AppleScript command
Blocks: 156078
Priority: -- → P3
Target Milestone: --- → Camino1.2
Component: General → OS Integration
QA Contact: winnie → os.integration
Blocks: 380580
No longer blocks: 380580
No longer blocks: 125419
Assignee: sfraser_bugs → nobody
Target Milestone: Camino1.6 → ---
Peeja, how hard (on a scale of 1-10) would it be to add support for another optional parameter to our dictionary? I can look into this if it's, say, about a 5 or less.
Hardware: Macintosh → All
From the Navigator 4.08 dictionary, complete with misspellings:

Standard URL suite	Mac URL standard, supported by many apps

GetURL v : Loads the URL (optionaly to disk)
  GetURL text : The url
    [to file specification] : file the URL should be loaded into
    [inside specifier] : Window the URL should be loaded to
    [from text] : Refererer, to be sent with the HTTP request

See also http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpfe/bootstrap/appleevents/nsAppleEvents.h&rev=1.5&mark=218-235#218 for the FourCCs of those.
Summary: support referrer property in OpenURL AppleScript command → support referrer property in GetURL AppleScript command
Attached patch WIP patch (obsolete) — Splinter Review
Comment 0 is really asking for about seven different things, as I see it. Since the summary was changed a long time ago to make this specifically about adding support for "referrer", I think we should do that here, and worry about the other six parameters in separate, UNCO bugs. (I really think most of them are fringe features that are of dubious utility.)

Fixing the referrer thing is fairly straightforward. However...

We need to modify MainController's showURL: method to accept a referrer: parameter. This is likewise pretty easy, although we should think about what we want to do in the event that an AppleScript asks Camino to open an already-open URL with a referrer property, since the reload: method doesn't know anything about referrers at all, and there are a whole lot of callers to reload: that would have to change if we changed that method signature.
Assignee: nobody → cl-bugs-new
Status: NEW → ASSIGNED
Yeah, I know. That's why it's a "WIP" patch, pending some discussion about the issues I raised in comment 6. I didn't expect it to compile, or at least I don't think I bothered trying to compile it. (I'd guess it's failing with an "unused variable" warning, since I don't do anything with |referrer| after declaring and initialising it.)
(In reply to Chris Lawson from comment #6)
> We need to modify MainController's showURL: method to accept a referrer:
> parameter. This is likewise pretty easy, although we should think about what
> we want to do in the event that an AppleScript asks Camino to open an
> already-open URL with a referrer property, since the reload: method doesn't
> know anything about referrers at all, and there are a whole lot of callers
> to reload: that would have to change if we changed that method signature.

FWIW, I think in the case where someone specifies a referrer, we should always treat that as a new request and open it in a new (tab|window), rather than reloading the existing tab with a new referrer (which isn't really a reload, anyway).  

And, for practical implementation purposes, I suspect that would be far easier, although it seems like |reload:| doesn't have that many callers nowadays: http://mxr.mozilla.org/camino/search?string=reload%3A&find=camino%2Fsrc%2F&tree=camino

Oh, but on deeper digging, we can't use |reload:|, anyway (unless we want to throw away the user-specified referrer entirely, which would upset people), because nsIWebNavigation's |Reload()| doesn't know anything at all about referrers.

So I think that the answer to the questions in comment 6 is to check and see if there's a non-nil referrer, and if so, load the page in a new (tab|window) with the referrer; if there's a nil referrer, check to see if it's already loaded and either reload and show or open a new foo like currently.

I suppose we could do something complicated like "see if it's already open, and, if so, get the BrowserWrapper for the URL and call loadURI: on it with the new referrer; if not, new foo", but I think likely consumers of "referrer" are going to be advanced users who'd expect/prefer a new foo rather than us finding-and-reloading an existing URL (which I suppose is a good thing for normal users and links from external apps).  In fact, I could use "referrer" in my toolbar scripts to work around the problem of my work-around for the inability to "make new tab" getting hijacked by an existing tab with the same URL…
(In reply to Smokey Ardisson (back-ish; no bugmail - do not email) from comment #9)

> So I think that the answer to the questions in comment 6 is to check and see
> if there's a non-nil referrer, and if so, load the page in a new
> (tab|window) with the referrer; if there's a nil referrer, check to see if
> it's already loaded and either reload and show or open a new foo like
> currently.

That seems reasonable. In pseudo-code:

if (referrer)
  open new instance using referrer
else
  do current behavior

cl
Attached patch Full patchSplinter Review
> That seems reasonable. In pseudo-code:
> 
> if (referrer)
>   open new instance using referrer
> else
>   do current behavior

The problem with the pseudo-code is that the |if (referrer)| case needs to reuse about 20 lies of "do current behavior".  I think it turns out to be a lot saner to just wrap the "look and see if the URL is currently open and, if so, show and reload it" code in an |if (!referrer)| and then share the last 20-some-odd-lines of the current behavior code.

So that's what this patch does.  I've gone through all the callers of |showURL:| and added nil referrers and have checked all of them (except Services, which is a pain to test) to verify they still behave.  I've also checked the 4 non-reload cases in the current code, with and without referrers.

I'm hoping Chris won't mind me stealing this from his, since he hasn't had a tree for a while ;-)
Assignee: cl-bugs-new2 → alqahira
Attachment #337596 - Attachment is obsolete: true
Attachment #579952 - Flags: superreview?(stuart.morgan+bugzilla)
Also, to test this, you can use the following AppleScript one-liner:

tell application "/path/to/patched/copy/of/Camino.app" to open location "http://www.ardisson.org/smokey/urltest" using referrer "http://apple.com/"

My 404 pages contain a line "If you came here from another site, you can click your browser’s Back button to go back _there_ (and maybe you can let that site’s webmaster know about the broken link)." you can use to see the referrer, if one is present.

(You can also wrap that AppleScript in osascript -e 'above-applescript' and run it from the Terminal.)
Comment on attachment 579952 [details] [diff] [review]
Full patch

sr=smorgan

Given that most calls don't pass a referrer it might make more sense to keep a showURL: method that just calls the new one with a nil referrer, but I'm fine with it either way.
Attachment #579952 - Flags: superreview?(stuart.morgan+bugzilla) → superreview+
For reference, here's the version of the patch that preserves the no-argument showURL:, per sr comments.

Since the only caller of showURL: that did care about passing a referrer is GetURLCommand, it does indeed make more sense to do it this way, and this is the version of the patch that's landing.
http://hg.mozilla.org/camino/rev/11c25585cd98

Thanks to Chris for the start on this one!
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: camino2.1.1? → camino2.1.1+
Resolution: --- → FIXED
Whiteboard: [camino-2.1.1]
You need to log in before you can comment on or make changes to this bug.