Skip to content

Conversation

shshaw
Copy link

@shshaw shshaw commented Sep 26, 2012

Only process hashtags if enabled in options, ensuring compatibility with other scripts using hashtags with the option hashTags : false

@Mottie
Copy link
Contributor

Mottie commented Oct 2, 2012

Actually that change only prevents setting up the hash tag regex. Line 636 is the line that prevents processing hashtags if the option is disabled.

if (o.hashTags) { base.setHash(page); }

So, I'm not sure why this change is necessary? Is there a bug?

@shshaw
Copy link
Author

shshaw commented Oct 3, 2012

It's related to issue #413: Anythingslider breaking with an equal in the hash. This commit is a bandaid that helped me get AnythingSlider working in my particular context.

Even with hashTags: false, when I used Hash Bang (#!) tags for another script I was getting a nondescript error in jQuery Uncaught Error: Syntax error, unrecognized expression: #!Kids that I finally tracked down to Anything Slider.

The code I committed fixed the error, but there is probably a better way to correctly fix the problem. I'm not sure what specifically in that is causing issues with #!, and that root issue should probably be looked at. My guess would be that AnythingSlider is evaluating the hashtags (even when they are disabled) and choking on non-alphanumeric characters in the hashtags before it even gets to Line 636.

@Mottie
Copy link
Contributor

Mottie commented Oct 3, 2012

Ok, thanks for the update. I'll look into this a bit more, but I most likely won't use the changes in this pull request as it breaks the functionality of the gotoHash function - which is probably where the problem is - but I don't have time to investigate it today.

@shshaw
Copy link
Author

shshaw commented Oct 3, 2012

Thanks for checking into it! No worries about using this pull request. It fixed the error for me, but I didn't test it with hashes enabled since I don't use that functionality.

Mottie added a commit that referenced this pull request Dec 7, 2012
@Mottie
Copy link
Contributor

Mottie commented Dec 7, 2012

Could you please verify that the latest version doesn't have this issue. Thanks!

@Mottie Mottie closed this Dec 7, 2012
@shshaw
Copy link
Author

shshaw commented Dec 19, 2012

Issue appears to be resolved with the latest version. Thanks!

@basememara
Copy link

There is still a bug in this. Try going to #!demo/slider and jQuery will crash because the selector is invalid. The regex test on line 716 should be !/#!?\/?/.test(h) instead of !/#!?\//.test(h).

Also you might want to check if the selector is even valid since it is arbitrary passed in by the end user. You can escape the selector so it is valid according to web standards:

"ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons (":"), and periods (".")."

So something like this will work:

selector = selector.replace(/([;&,\.\+\*\~':"\!\^#$%@\[\]\(\)=>\|])/g, '\\$1');

@basememara
Copy link

Something's still wacky going on. Shouldn't hashTag: false go around all this? I can't use the slider with http://canjs.us.

@shshaw
Copy link
Author

shshaw commented Dec 20, 2012

That was my initial thought. Contain all of the hash tag processing with if ( o.hashTags === true ) { } to just allow for disabling hash tags completely.

@Mottie
Copy link
Contributor

Mottie commented Dec 20, 2012

Hmm, well I left in the ability to pass a hash tag to a page (e.g. simple.html#&panel1-3) to set the slide ignoring the hash tag setting. If you think it's a better idea to disable it completely, I can update the plugin.

@basememara
Copy link

I believe it should be left out completely if hashTag is off because it will conflict will all JavaScript history routers (jQuery-BBQ, jQuery Mobile, CanJS, Backbone, etc). AnythingSlider will keep trying to interpret stuff that history routers are setting which is not intended. If someone wants to change the slider to a certain panel, they can either enable hashTags in AnythingSlider, or disable hashTag and change the slider through the API manually.

@Mottie
Copy link
Contributor

Mottie commented Dec 21, 2012

Ok, I just updated the repo to version 1.8.8. The hash will now be completely ignored if hashTags is set to false.

@basememara
Copy link

Awesome works great! Thanks!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants