New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loadPage: Selecting by ID not special-character-friendly #5949

Closed
gabrielschulhof opened this Issue Apr 30, 2013 · 1 comment

Comments

Projects
None yet
1 participant
@gabrielschulhof
Contributor

gabrielschulhof commented Apr 30, 2013

Context: js/jquery.mobile.navigation.js:

        // If we failed to find the page, check to see if the url is a
        // reference to an embedded page. If so, it may have been dynamically
        // injected by a developer, in which case it would be lacking a data-url
        // attribute and in need of enhancement.
        if ( page.length === 0 && dataUrl && !path.isPath( dataUrl ) ) {
            page = settings.pageContainer.children( "#" + dataUrl )
                .attr( "data-" + $.mobile.ns + "url", dataUrl )
                .jqmData( "url", dataUrl );
        }

Instead of .children( "#" + dataUrl ) we need to use document.getElementById(), because that function can handle the weird characters that may be present in the ID, but then we still need to make sure that the returned element is a child of settings.pageContainer.

The workaround we should advocate for now is for devs to always add the data-url attribute to their dynamically created pages. They need not worry about adding the data-url attribute to static pages, because that's added by us upon init.

I didn't want to make the change because of @johnbender's nav refactor work. It would make merging harder afterwards.

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Apr 30, 2013

Contributor

I decided instead to centralize the implementation of the conversion of hash to selector (thereby making a minor modification to js/jquery.mobile.navigation.js), for two reasons:

  1. It makes no sense to place escaped hashes into the DOM as the custom select widget was doing before. The hashes need to appear unescaped in the DOM, and they should be escaped at the point where they are passed to the jQuery constructor.
  2. Using getElementById() doesn't make sense in places other than init, because in all other places where a hash that contains special characters presents a problem, we are using selectors not wrt. the document, but wrt. a given element (settings.pageContainer for dialogs and the current page for popups). document.getElementById() cannot be limited to a subtree (unlike jQuery objects), so it's better to create a valid jQuery selector.
Contributor

gabrielschulhof commented Apr 30, 2013

I decided instead to centralize the implementation of the conversion of hash to selector (thereby making a minor modification to js/jquery.mobile.navigation.js), for two reasons:

  1. It makes no sense to place escaped hashes into the DOM as the custom select widget was doing before. The hashes need to appear unescaped in the DOM, and they should be escaped at the point where they are passed to the jQuery constructor.
  2. Using getElementById() doesn't make sense in places other than init, because in all other places where a hash that contains special characters presents a problem, we are using selectors not wrt. the document, but wrt. a given element (settings.pageContainer for dialogs and the current page for popups). document.getElementById() cannot be limited to a subtree (unlike jQuery objects), so it's better to create a valid jQuery selector.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment