PageContainer: dynamically injected pages will not be removed from DOM automatically on hide

[frequent]

Demo

In Firefox:

• Check DOM
• dashboard is first page, services is dynamically generated from the deeplink
• Open panel, click networks
• network page is dynamically generated and transitioned to
• In Firebug, is still in the DOM services although it should have been removed
• Open panel, click servers
• again, page is generated dynamically and transitioned to, but the previous page is not removed.

I checked the pageContainer widget.

Dynamically injected content is missing a call to page.page( "bindRemove" );
Normally this is called inside the pageContainer widget _include() method, which is called on _loadSuccess(), which will only be called when an Ajax call is made to fetch a page - so not in the case of dynamic injection.

So if I intercept the changePage on pagebeforechange, generate a dynamic page, append it to the DOM and then trigger a new changePage to that page, I'm stuck without the remove mechanism.

To fix inside pageContainer widget, load method, I add:

      if ( content.length && !settings.reload ) {
        this._enhance( content, settings.role );
        // FIX: add remove bindings
        content.page( "bindRemove" );
        ...

Question is whether dynamic injection also means dynamic cleanup, but I think the hide events should also trigger regardless of "who" is handling DOM maintenance.

[gabrielschulhof]

Your demo doesn't work in Chrome. Chrome just shows a blank page. I wouldn't be surprised though. Navigation during startup doesn't work in Chrome.

[frequent]

@gabrielschulhof: please use Firefox, because the demo uses a custom promise() implementation which we have not fixed on Chrome :-)

[gabrielschulhof]

OK, but this should be reproducible with a simple jsbin as well ...

[frequent]

@gabrielschulhof - I try to make one btw lunch and dessert :-)

[gabrielschulhof]

Simple test case

1.3.2: http://jsbin.com/ofuhaw/704/
master: http://jsbin.com/ofuhaw/705/

[gabrielschulhof]

Here's a workaround:

http://jsbin.com/ofuhaw/706/

[gabrielschulhof]

The funny thing is that if we fix this using @frequent's PR, then we will have to make sure that bindRemove() only gets called once per page and that a second call has no effect, because other people may have stumbled across this problem and applied my workaround. For those people, bindRemove() will certainly get called at least twice for each generated page - once by their own code, and once by ours.

[gabrielschulhof]

Another aspect that may be worth investigating as we fix this bug is why we're calling bindRemove() from pagecontainer at all. We could just as easily call bindRemove() from the page's _create() method and remove all bindRemove() calls from pagecontainer. Then page removal would be entirely separated from navigation.

[jaspermdegroot]

@gabrielschulhof - You closed this ticket but the PR #6657 is still open. I guess you closed by accident so I am going to re-open. Let me know if I guessed wrong.

[arschmitz]

Personally I don't think we should remove dynamically injected pages. These should be treated the same as any other internal page. Meaning they should be left in the DOM for the dev to deal with it. Otherwise we would have to store a list of all the internal pages that existed when the framework spins up, to be able to tell the difference between an internal page and a dynamically injected page.

[frequent]

@arschmitz:
while I agree to remove by hand I thought data-external-page=true was the flag set on pages pulled in (dynamic or not) which triggers removal of external pages upon user departure. I initially hoped I just add the attribute on the dynamic page and save myself a custom handler like:

      cleanupHandler = function (e) {
        var page, id, is_enhanced;

        page = e.target;
        id = page.id;
        is_enhanced = $(page).data("mobilePage") || $(page).page("instance");

        if (page.getAttribute('data-external-page')) {
          if (is_enhanced) {
            $(this).page('destroy').remove();
          }
        }
      };

[arschmitz]

@frequent one thing that comes to mind in all of this is I am planning on moving all page removal logic to pagecontainer as a page isn't going to remove it self so the logic should be where its getting removed.

So if we do something along those lines it should be in conjunction with this.

[arschmitz]

going to bump this to 1.6 where we will handle more nav issues