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

jqmDataRE kills SAP URLs with brackets #4849

Closed
cawoodm opened this Issue Aug 15, 2012 · 4 comments

Comments

Projects
None yet
2 participants
@cawoodm

cawoodm commented Aug 15, 2012

In jquery.mobile-1.1.1.js on line 1543 there is the following:
jqmDataRE = /:jqmData(([^)]*))/g;

This basically searches a string from :jqmData till the end or next closing bracket .

However, some URLs (like those of SAP BSPs) have brackets in them. Example:
http://mysapserver.com/abc(bD1lbiZjPTEwMA==)/bsp/cat/controller.do

Such URLs get destroyed by the above RegEx.
:jqmData(url='/zlgs(bD1lbiZjPTEwMA==)/bsp/cat/controller.do?a=0000603565')
becomes
[data-url='/zlgs(bD1lbiZjPTEwMA==]/bsp/cat/controller.do?a=0000603566')

which is an invalid jQuery selector.

Basically the jqmDataRE regex assumes URLs do not have brackets in them and SAP URLs always have brackets in them.

I have found searching for a space to be better:

jqmDataRE = /:jqmData(([^ ]*))/g;

@johnbender

This comment has been minimized.

Show comment
Hide comment
@johnbender

johnbender Aug 15, 2012

Contributor

@cawoodm

Thanks for logging the issue.

We clearly need to deal with this case given that the paren is a valid URL char per the spec. Unfortunately your proposed change won't work because we may (now or in the future) have data-namesspace-attribute selectors that contain whitespace. Eg

$( ":jqmData(text='foo bar baz')" );

It looks like we'll have to do a manual namespace concat in the places where we're selecting on the URL. I should have a patch for this some time today.

Contributor

johnbender commented Aug 15, 2012

@cawoodm

Thanks for logging the issue.

We clearly need to deal with this case given that the paren is a valid URL char per the spec. Unfortunately your proposed change won't work because we may (now or in the future) have data-namesspace-attribute selectors that contain whitespace. Eg

$( ":jqmData(text='foo bar baz')" );

It looks like we'll have to do a manual namespace concat in the places where we're selecting on the URL. I should have a patch for this some time today.

@ghost ghost assigned johnbender Aug 15, 2012

@johnbender johnbender reopened this Aug 15, 2012

@johnbender

This comment has been minimized.

Show comment
Hide comment
@johnbender

johnbender Aug 15, 2012

Contributor

Pushed to master.

@cawoodm

Can you verify that this works for you by including http://jquerymobile.com/branches/1.1-stable/js/ for JQM?

Contributor

johnbender commented Aug 15, 2012

Pushed to master.

@cawoodm

Can you verify that this works for you by including http://jquerymobile.com/branches/1.1-stable/js/ for JQM?

@cawoodm

This comment has been minimized.

Show comment
Hide comment
@cawoodm

cawoodm Aug 16, 2012

Yes, thanks for the amazingly fast fix!

cawoodm commented Aug 16, 2012

Yes, thanks for the amazingly fast fix!

@johnbender

This comment has been minimized.

Show comment
Hide comment
@johnbender

johnbender Aug 17, 2012

Contributor

@cawoodm

No problem, I'm sorry we don't have something more comprehensive for the selector. All I could do was add docs. This will go out with the 1.2 release candidate and 1.1.2.

Contributor

johnbender commented Aug 17, 2012

@cawoodm

No problem, I'm sorry we don't have something more comprehensive for the selector. All I could do was add docs. This will go out with the 1.2 release candidate and 1.1.2.

arschmitz added a commit to arschmitz/jquery-mobile that referenced this issue Oct 16, 2012

Handle urls with parens properly
The regular expression used to parse the jqmData psuedo selector restricts the
use of parentheses which are valid in urls. This breaks data-ns-url selection.
The fix is to avoid the pseudo selector. Fixes #4849
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment