Skip to content
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

ui.tabs _isLocal fails if url contains username:password@ #2213

Open
IAMJDA opened this issue Feb 28, 2024 · 9 comments
Open

ui.tabs _isLocal fails if url contains username:password@ #2213

IAMJDA opened this issue Feb 28, 2024 · 9 comments

Comments

@IAMJDA
Copy link

IAMJDA commented Feb 28, 2024

I had some really ugly problems with some jquery ui tab component and could pin it down to a wrong calculation in _isLocal.

decodeURIComponent(e) returns https://username:password@serverfqdn/path/file
decodeURIComponent(i) returns https://serverfqdn/path/file

These two are compared and the difference causes the return value to be false instead of true.
Therefore, the whole render logic breaks...

@mgol
Copy link
Member

mgol commented Mar 5, 2024

Thanks for the report. Does the issue you describe exist when jQuery UI 1.12.1 is used or only with jQuery UI 1.13.0 or newer?

Also, please provide a runnable test case on a platform like JS Bin.

@IAMJDA
Copy link
Author

IAMJDA commented Mar 13, 2024

Sorry, I'm not a JavaScript dev. The function is depending on username + password being in the url, so a 1:1 example with any code plattform is unlikely to work.

The code in latest git seems to be the same. The culprit is that the anchor and location can have different values for the same URL.

@mgol
Copy link
Member

mgol commented Mar 13, 2024

I understand but we cannot do much without a test case.

One thing you may be able to help with is saying whether jQuery UI 1.12.1 exhibits the same issue. You can find links to all releases on https://releases.jquery.com.

@IAMJDA
Copy link
Author

IAMJDA commented Mar 13, 2024

Please check https://jsbin.com/rinenupiba/edit?html,js,output

I faked _isLocal with user+password in location:
locationUrl = location.href.replace("://", "://test:test@").replace( rhash, "" );

Rendering of the page will change if you revert that line.

@mgol
Copy link
Member

mgol commented Mar 14, 2024

It's not clear to me what the JS Bin is supposed to show. Can you use the stock version of jQuery UI without repeating its source in the JS pane and explain exactly what you expect to happen in this test case and what is happening instead?

@mgol
Copy link
Member

mgol commented Mar 14, 2024

BTW, the _isLocal definition is identical between 1.12.1 & 1.13.2. Whatever the issue is, since it is already in 1.12, given limited team resources it's not likely to be fixed by the UI team; see the project status at https://blog.jqueryui.com/2021/10/jquery-maintainers-update-and-transition-jquery-ui-as-part-of-overall-modernization-efforts/. PRs are welcome if they're not too complex and contain tests.

@IAMJDA
Copy link
Author

IAMJDA commented Mar 15, 2024

It's not clear to me what the JS Bin is supposed to show. Can you use the stock version of jQuery UI without repeating its source in the JS pane and explain exactly what you expect to happen in this test case and what is happening instead?

The way browser and JS bin work, make it impossible to reproduce it (at least for me). I cannot call JS bin like https://user:[email protected].

The problem is quite easy:

anchor.href behaves differently than location.href - it just does not contain the user:password part.

@mgol
Copy link
Member

mgol commented Mar 15, 2024

OK, my last comment about issues shared with 1.12 still stands, though.

@Pascal76
Copy link

Pascal76 commented Mar 17, 2024

Same issue here :

#tabs- links causes the whole page to be reloaded for ever on the tabs :(
I am on 1.13.2 build
Same issue with 1.12.1

When I copy the URL, remove the "username:password@" part and paste it on it on the browser, then it works as expected.

My (bad hack) solution is to open the page through a popup, with an absolute link and without the credentials on it (not needed when already authenticated)

@mgol mgol removed the Needs info label May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants