Ajax and "base" tag

[jguyomard]

Ajax didn't work when an original "base" tag is defined in HTML... This patch fixes this, when $.support.dynamicBaseTag is set to true.

[jblas]

@jguyomard

Do you have a test case that demonstrates the problem you are trying to fix? In your case, what is the url being calculated?

[jguyomard]

Ok. i've a test that demonstrates the problem:

With the current version of JQM, when an original base tag is defined in HTML, it doesnt works:
http://labs.julien.guyomard.org/jqm/base/original.base/

With my version, it works:
http://labs.julien.guyomard.org/jqm/base/commit.base/

Others tests (it still works when no original base tag is defined!) are available here:
http://labs.julien.guyomard.org/jqm/base/

[jblas]

@jguyomard

Wow looking at your patch forced me to look at the baseTagTest() implementation. I really messed that one up cause this all used to work. Anyways, the current incarnation of baseTagTest() doesn't even test the dynamic changing of a base tag so I re-wrote it so it did. It looks like this:

function baseTagTest()
{
    var defaultBase = location.href,
        base = $( "head base" ),
        fauxEle,
        rebasedDir,
        href,
        hrefRebased;

    // If the document already has a <base> tag in the head, use it for our tests.
    // Otherwise, generate a base tag, set it's base to the document href by default.

    if ( base.length ) {
        defaultBase = base[ 0 ].href;
    } else {
        base = fauxEle = $( "<base>", { "href": defaultBase }).appendTo( "head" );
    }

    link = $( "<a href='testurl' />" ).prependTo( fakeBody );

    href = link[ 0 ].href;

    // Extract the path directory from the base so we can append a fake sub-directory
    // to it for our test below.

    rebasedDir = base[ 0 ].href.replace(/([^\/\?#]+)?(\?[^#]*)?(#.*)?$/, "") + "ui-dir/";

    // Change the href of the base tag so we can test if the browser supports
    // dynamic href manipulation.

    base[ 0 ].href = rebasedDir;

    // Refetch the href of the link. The href returned here should be relative
    // to the rebasedDir.

    hrefRebased = link[ 0 ].href;

    // If we added a base tag to the document remove it. If we used an existing
    // one, reset the href back to what it was before we started the test.

    if ( fauxEle ) {
        fauxEle.remove();
    } else {
        base[ 0 ].href = defaultBase;
    }

    return href != hrefRebased && hrefRebased.indexOf( rebasedDir ) != -1;
}

So now my question, is why was it necessary to change the getClosestBaseUrl() to check $.support.dynamicBaseTag?

[jguyomard]

why was it necessary to change the getClosestBaseUrl() to check $.support.dynamicBaseTag?

It's not to check $.support.dynamicBaseTag. It's because, without this change, when an original base tag is defined in HTML, getClosestBaseUrl() doesn't return the right url...

[jblas]

Ok, so I had some time today to look into this more. There are actually several problems.

1. baseTagTest() as implemented in 1.0 is not returning the correct support value in the case the document contains an existing tag.
2. baseTagTest() as implemented in 1.0 does not test the actually dynamic change of the base tag in the case the document contains an existing tag.
3. getClosestBaseUrl() assumes that the data-url path for a given page is the base when in-fact it may not be.
4. External documents that are loaded as pages can contain a base tag that uses a DIFFERENT path from the application document that loaded it with AJAX.

The baseTagTest() re-write I posted a few comments back addresses both #1 and #2.

@jguyomard attempted to fix #3 by forcing the URL to be used to be the base if $.support.dynamicBaseTag was true, but the fix assumes that all external documents will have the same base, when in fact they won't, especially in the case where no base tag is ever used, but the platform/device actually supports dynamicBaseTag.

#4 is a known issue. We don't support the loading of external pages that specify their own tag, which I hope to fix during the 1.x timeframe. I believe there is an actual issue already reported about it.

I think a better interim fix for #3, until we can add support for #4, is to modify getClosestBaseUrl() to do this check:

if ( !url || !path.isPath( url ) || path.isFirstPageUrl( url ) ) {

That way we can get the case where the application document contains a and the external pages don't, working.

The alternative, would be to assume that all external documents pulled in are assumed to have the exact same path which I think @jguyomard was thinking and instead check:

    if ( !url || !path.isPath( url ) || documentBaseDiffers ) {

@toddparker and @johnbender, thoughts?

[toddparker]

I don't think it's safe to assume that both have the same base tag for #3 so we should try to make this as robust as we can.

[jblas]

@toddparker

Right, which is why I was favoring the first example where we limit the scope to the firstPage in the interim. We really do need to fix #4 which would also cover #3.

[toddparker]

Makes sense. So you recommend fixing 1-3 sooner, but try to tackle 4 soon?

[jblas]

@toddparker

Yes.

[jguyomard]

Hi,

@jblas :

especially in the case where no base tag is ever used, but the platform/device actually supports dynamicBaseTag

Ok, you're right, my solution doesn't work if no base tag is ever defined in HTML, and if all external documents don't have the same base... I made a test here:
http://labs.julien.guyomard.org/jqm/base/commit.nobase.dir/index.htm

I made others tests, your solution works when no base tag is defined in HTML:
http://labs.julien.guyomard.org/jqm/base/jblas.nobase.dir/index.htm

...but does't work when a base tag is defined (link in ajax.htm doesn't work):
http://labs.julien.guyomard.org/jqm/base/jblas.base/index.htm

[jguyomard]

@jblas @toddparker

I made an other commit. Now, it works when an original base tag is defined in HTML AND it works if no base tag is ever defined in HTML:

• with an original base tag: http://labs.julien.guyomard.org/jqm/base/commitv2.base/
• without any base tag: http://labs.julien.guyomard.org/jqm/base/commitv2.nobase.dir/

All my tests are here: http://labs.julien.guyomard.org/jqm/base/

[jguyomard]

Arf, my solution doesn't work with images contained in page loaded by ajax... (when an original base tag is defined in HTML and src attribute is relative)...

Another solution is to convert all relative url to absolute url, when a page is loaded by ajax. Something like that in $.mobile.loadPage > $.ajax > success :

if ( $.support.dynamicBaseTag && $base.length ) {
    page.find( "[src], link[href], a[rel='external'], :jqmData(ajax='false'), a[target]" ).each(function() {
        var thisAttr = $( this ).is( '[href]' ) ? 'href' : $(this).is('[src]') ? 'src' : 'action';
        var thisUrl = $( this ).attr( thisAttr );
        if( !/^https?:/.test( thisUrl ) ) {
            $( this ).attr( thisAttr, documentBase.hrefNoHash + thisUrl );
        }
    });
}

Seems to work everywhere...

[jguyomard]

Hum, ok ; i've a better solution to correct the problem mentioned in my previous reply.

In $.mobile.loadPage > $.ajax > success, you have :

if ( base ) {
    base.set( fileUrl );
}

So, you modify original base tag with the url of the page loaded with by ajax...
If you remove/comment this lines, it works.

[johnbender]

This makes the whole branch a no-op. If that's the intention maybe just remove it?

[jblas]

If I'm not mistaken, that line is necessary to make sure that images within the page load properly. The problem here is that I think the base should be set to the fileUrl, or in the case where the page we just loaded contains a base tag, it should be set to that base tag path.

[johnbender]

@jblas

Is this something we should convert into an issue and take it from there internally? It seems like there's a lot to consider. Otherwise I'll have to defer to you on merging this in.

[jblas]

@johnbender

Yes, there should definitely be an issue filed for this stuff, with the points I mentioned above with what is wrong today.

I'm a bit slammed at the moment, otherwise I would jump on looking at this.

[johnbender]

@jguyomard

There are a whole host of issues at play here and we'll probably end up handling this internally as a result. Thanks for your pull request, we wouldn't have gotten this far without it!

@jblas @toddparker

See #3978.