Teach elgg/Ajax to work with absolute URLs #9564

Closed
hypeJunction opened this Issue Mar 24, 2016 · 6 comments

Comments

Projects
None yet
2 participants
@hypeJunction
Contributor

hypeJunction commented Mar 24, 2016

When we render DOM elements we tend to normalize URLs and client side we use $.attr() to retrieve those URLs. elgg/Ajax#path is not really suitable unless we denormalize the URL into path and query data.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Mar 24, 2016

Member

Right, the path() call works but the wrong hook types will be used.

  • path('http://example.org/elgg/foo') should be interpreted as path('foo').
  • What about path('/elgg/foo')?
  • What about queries and fragments?

Here's my thinking: For all methods, queries are passed through to the underlying XHR URL, but neither query nor fragment go in hook types.

For the hook type in path() and action(), we strip the leading site URL, and then strip any remaining leading /. So /elgg/foo is interpreted as elgg/foo.

So for more reliable usage, devs should use $.prop('href') instead of attr, that way always getting a full URL.

Member

mrclay commented Mar 24, 2016

Right, the path() call works but the wrong hook types will be used.

  • path('http://example.org/elgg/foo') should be interpreted as path('foo').
  • What about path('/elgg/foo')?
  • What about queries and fragments?

Here's my thinking: For all methods, queries are passed through to the underlying XHR URL, but neither query nor fragment go in hook types.

For the hook type in path() and action(), we strip the leading site URL, and then strip any remaining leading /. So /elgg/foo is interpreted as elgg/foo.

So for more reliable usage, devs should use $.prop('href') instead of attr, that way always getting a full URL.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Mar 24, 2016

Member

And of course in action() we don't need to decorate options.data if the query has CSRF tokens.

Member

mrclay commented Mar 24, 2016

And of course in action() we don't need to decorate options.data if the query has CSRF tokens.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Mar 24, 2016

Member

Technically someone could be tying hooks to path:/foo and this would break by normalizing to path:foo but I think that's a risk we have to take.

Member

mrclay commented Mar 24, 2016

Technically someone could be tying hooks to path:/foo and this would break by normalizing to path:foo but I think that's a risk we have to take.

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Mar 24, 2016

Contributor

I agree that we should pass normalized URL to fetch and denormalize it for hook names. Or maybe it's just easier to forget about granular hooks and let callbacks sniff the URL/path, as we do in PHP.

Contributor

hypeJunction commented Mar 24, 2016

I agree that we should pass normalized URL to fetch and denormalize it for hook names. Or maybe it's just easier to forget about granular hooks and let callbacks sniff the URL/path, as we do in PHP.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Mar 24, 2016

Member

Hook handers can already register for all, and/or sniff the URL from params.options.url.

Member

mrclay commented Mar 24, 2016

Hook handers can already register for all, and/or sniff the URL from params.options.url.

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 25, 2016

feature(ajax): better elgg/Ajax handling of form data and URLs
As `jQuery.serialize` is often used to set post data, this change allows a
string form of `options.data` to be passed through. In this case, however,
we do not trigger the request hook, as handlers expect an object.

Because we want requests to be hookable, we nudge devs toward a new `objectify`
method, which "serializes" to an object so it can be passed through the hook.

All methods now accept a query string that gets passed to the underlying URL,
but does not appear in the hook types. `action()` and `path()` accept absolute
site URLs, as one might get from `$anchor.prop('href')`.

In transitioning code to elgg/Ajax, it may be necessary to detect whether an
ajax request has come from it. For this we add `elgg_is_xhr2()`.

Fixes #9534
Fixes #9564
@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Mar 25, 2016

Member

I'm rolling this into #9547

Member

mrclay commented Mar 25, 2016

I'm rolling this into #9547

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 26, 2016

feature(ajax): better elgg/Ajax handling of form data and URLs
As `jQuery.serialize` is often used to set post data, this change allows a
string form of `options.data` to be passed through. In this case, however,
we do not trigger the request hook, as handlers expect an object.

Because we want requests to be hookable, we nudge devs toward a new `objectify`
method, which "serializes" to an object so it can be passed through the hook.

All methods now accept a query string that gets passed to the underlying URL,
but does not appear in the hook types. `action()`, `path()`, and `form()` accept
absolute site URLs, as one might get from `$anchor.prop('href')`.

In transitioning code to elgg/Ajax, it may be necessary to detect whether an
ajax request has come from it. For this we add `elgg_is_xhr2()`.

Fixes #9534
Fixes #9564

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 26, 2016

feature(ajax): better elgg/Ajax handling of form data and URLs
As `jQuery.serialize` is often used to set post data, this change allows a
string form of `options.data` to be passed through. In this case, however,
we do not trigger the request hook, as handlers expect an object.

Because we want requests to be hookable, we nudge devs toward a new `objectify`
method, which "serializes" to an object so it can be passed through the hook.

All methods now accept a query string that gets passed to the underlying URL,
but does not appear in the hook types. `action()`, `path()`, and `form()` accept
absolute site URLs, as one might get from `$anchor.prop('href')`.

In transitioning code to elgg/Ajax, it may be necessary to detect whether an
ajax request has come from it. For this we add `elgg_is_xhr2()`.

Fixes #9534
Fixes #9564

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 26, 2016

feature(ajax): better elgg/Ajax handling of form data and URLs
As `jQuery.serialize` is often used to set post data, this change allows a
string form of `options.data` to be passed through. In this case, however,
we do not trigger the request hook, as handlers expect an object.

Because we want requests to be hookable, we nudge devs toward a new `objectify`
method, which "serializes" to an object so it can be passed through the hook.

All methods now accept a query string that gets passed to the underlying URL,
but does not appear in the hook types. `action()`, `path()`, and `form()` accept
absolute site URLs, as one might get from `$anchor.prop('href')`.

In transitioning code to elgg/Ajax, it may be necessary to detect whether an
ajax request has come from it. For this we add `elgg_is_xhr2()`.

Fixes #9534
Fixes #9564

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 26, 2016

feature(ajax): better elgg/Ajax handling of form data and URLs
As `jQuery.serialize` is often used to set post data, this change allows a
string form of `options.data` to be passed through. In this case, however,
we do not trigger the request hook, as handlers expect an object.

Because we want requests to be hookable, we nudge devs toward a new `objectify`
method, which "serializes" to an object so it can be passed through the hook.

All methods now accept a query string that gets passed to the underlying URL,
but does not appear in the hook types. `action()`, `path()`, and `form()` accept
absolute site URLs, as one might get from `$anchor.prop('href')`.

Fixes #9534
Fixes #9564

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 26, 2016

feature(ajax): better elgg/Ajax handling of form data and URLs
As `jQuery.serialize` is often used to set post data, this change allows a
string form of `options.data` to be passed through. In this case, however,
we do not trigger the request hook, as handlers expect an object.

Because we want requests to be hookable, we nudge devs toward a new `objectify`
method, which "serializes" to an object so it can be passed through the hook.

All methods now accept a query string that gets passed to the underlying URL,
but does not appear in the hook types. `action()`, `path()`, and `form()` accept
absolute site URLs, as one might get from `$anchor.prop('href')`.

Makes sure built-in response handler fires early, to prevent plugin handlers from
seeing these values.

Fixes #9534
Fixes #9564

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 26, 2016

feature(ajax): better elgg/Ajax handling of form data and URLs
As `jQuery.serialize` is often used to set post data, this change allows a
string form of `options.data` to be passed through. In this case, however,
we do not trigger the request hook, as handlers expect an object.

Because we want requests to be hookable, we nudge devs toward a new `objectify`
method, which "serializes" to an object so it can be passed through the hook.

All methods now accept a query string that gets passed to the underlying URL,
but does not appear in the hook types. `action()`, `path()`, and `form()` accept
absolute site URLs, as one might get from `$anchor.prop('href')`.

Fixes #9534
Fixes #9564

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 26, 2016

feature(ajax): better elgg/Ajax handling of form data and URLs
As `jQuery.serialize` is often used to set post data, this change allows a
string form of `options.data` to be passed through. In this case, however,
we do not trigger the request hook, as handlers expect an object.

Because we want requests to be hookable, we nudge devs toward a new `objectify`
method, which "serializes" to an object so it can be passed through the hook.

All methods now accept a query string that gets passed to the underlying URL,
but does not appear in the hook types. `action()`, `path()`, and `form()` accept
absolute site URLs, as one might get from `$anchor.prop('href')`.

Fixes #9534
Fixes #9564

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 26, 2016

feature(ajax): better elgg/Ajax handling of form data and URLs
As `jQuery.serialize` is often used to set post data, this change allows a
string form of `options.data` to be passed through. In this case, however,
we do not trigger the request hook, as handlers expect an object.

Because we want requests to be hookable, we nudge devs toward a new `objectify`
method, which "serializes" to an object so it can be passed through the hook.

All methods now accept a query string that gets passed to the underlying URL,
but does not appear in the hook types. `action()` and `path()` accept
absolute site URLs, as one might get from `$anchor.prop('href')`.

Fixes #9534
Fixes #9564

@mrclay mrclay closed this in #9547 Mar 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment