fixed entwine for usage with jquery 1.12.x and 2.2.x

[andrelohmann]

Hi Hamisch, I had some errors regarding your $.browser.msie fix on jquery 1.12.x and 2.2.x so I fixed it.

Can you please review and merge my pull request, to have that fix to be part of the next silverstripe security release?

kind regards

[sminnee]

Thanks @andrelohmann,

Your patch seems to revolve around the $.browser.msie being set to undefined. From a quick search I can see that this has been deprecated in more recent versions of jQuery.

Your change means that focusInBubbles will be set to false on MSIE + jQuery >= 1.9, whereas it is supposed to be set to true. Have you tested what impact this has?

Also, can you please update the PR to amend the sources and use the standard package process to re-build the dist files (which is just calling build.sh).

[bummzack]

Not really sure what that patch does? This line also covers the undefined case?

$.support.focusinBubbles = !!($.browser.msie);

What were the errors you had? I think you should check for $.browser being undefined, since that's what has been deprecated? Also see #32

[andrelohmann]

on the current chrome and Firefox, the line "!!($.browser.msie);" fails, as $.browser.msie does not exist.

[bummzack]

@andrelohmann Yeah, I just updated my comment above.
Possible fix could be:

$.support.focusinBubbles = !!($.browser) && !$.browser.msie;

[andrelohmann]

Alright, I took over @bummzack 's suggestion and it worked perfectly

[sminnee]

There are still changes in dist files that aren't covered in src files. You should never edit a dist file by hand, they should only be modified by running build.sh.

The change itself seems reasonable, although I don't have confidence that this will make Entwine fully functional with newer versions of jQuery.

[sminnee]

@hafriedlander are you open to adding some other committers to this project? It would be good to add a travis config.

[bummzack]

I'm not sure how this was tested, but I don't think the added code works as expected? Currently it's:

!!($.browser.msie) && !$.browser.msie;

But that's twice checking for a property of $.browser, although $.browser itself was removed… it should read:

!!($.browser) && !$.browser.msie;

[andrelohmann]

@bummzack @sminnee I updated my PR again. Btw. I just run the build.sh script and thats what is alternating dist.

[lekoala]

I know this is an old issue but maybe someday it will be fixed for those using entwine and want a modern jquery

As far as I understand, focusin bubbles in modern browsers as per specs
https://w3c.github.io/uievents/#focusin

Therefore, this should be enough

if ($.support.focusinBubbles === undefined) {
    $.support.focusinBubbles = true; // old browsers just have to let go
}

Is there a reason not to update entwine to support modern jquery? I had to use my own customized version of entwine in multiple modules and that feels like a waste. It could be just merged into the core because we are going to need jquery in SilverStripe as long as bootstrap 4 is used.

[sminnee]

Yeah we may want to fork this repo back to the Silverstripe org, with a view to maintaining it at least for the CMS4 lifespan.