Sorting via keyboard keypress in Internet Explorer doesn't work correctly

Sorting via keyboard keypress in Internet Explorer doesn't work correctly

kdorffkdorff Posts: 16Questions: 0Answers: 0
edited August 2013 in DataTables 1.9
My application uses DataTables 1.9.4. My app was being reviewed by our accessibility team and they discovered that when using Internet Explorer, if you press TAB to go to a header cell and press return, it submits the form instead of sorting the column

I verified that the press return to sort worked CORRECTLY in:
* Chrome (28.0.1500.95), Mac
* Chrome (28.0.1500.95 m), Windows
* Firefox (22 and 23), Mac
* Firefox (20.0.1, 22, and 23), Windows
* Safari (5.1.7), Windows
* Safari (6.0.5), Mac

I verified their report of this problem with Internet Explorer 10 (10.0.9200.16635) in Windows 8. They were likely using either IE9 or IE10 in Windows 7 (I don't immediately know, but if it makes a difference I can find out).

Replies

  • kdorffkdorff Posts: 16Questions: 0Answers: 0
    edited August 2013
    I have created a simple jsFiddle demonstration of what is happening

    http://jsfiddle.net/kdorff/NPYPv/12/

    Run it in IE, see the table (bottom right), click on the search filter, tab to a column header. Press enter/return.

    I see that the form sorted THEN the form is ALSO submitted.

    If I comment out the submit buttons on the form, the submission doesn't occur, the sorting happens as normal. I can places the submit buttons either or both inside / outside the table and it makes no difference.

    If I run this sample example in other browsers, sorting the column with the keyboard, it doesn't submit the form but sorts and behaves correctly.
  • allanallan Posts: 54,880Questions: 1Answers: 8,601 Site admin
    Oh my - that's bizarre! I'm not sure that is something that I can fix in DataTables I'm afraid - it looks like an IE error.

    A possible workaround might be to check for what element is being activated ( `e.target` might tell you that) and then not submitting if its not the form. Unfortunately I don't actually have an IE machine to hand at the moment to try it out.

    Allan
  • kdorffkdorff Posts: 16Questions: 0Answers: 0
    edited August 2013
    I'm not completely sure what to look for. When I do either (where "e" is the submit click event)

    jQuery(e.target).prop('tagName')
    jQuery(e.originalEvent.target).prop('tagName')

    these both return "FORM" if I press return on the header OR if I click a button, so I'm not exactly sure what to test and the e object is somewhat huge.

    My "best guess" is that whatever event is triggering the sort is missing a return false at the end and Chrome/Safari don't care but IE not seeing the return value of false as an OK to submit. But that is 100% cure conjecture.

    I'm happy to work around this in my code with a check on submit if "msie" and what caused the submit and I can discriminate what caused the submit.
  • kdorffkdorff Posts: 16Questions: 0Answers: 0
    edited August 2013
    I added one line to the code which seems to have fixed the problem

    [code]
    function _fnSortAttachListener ( oSettings, nNode, iDataIndex, fnCallback )
    {
    _fnBindAction( nNode, {}, function (e) {
    e.preventDefault();

    /* If the column is not sortable - don't to anything */
    if ( oSettings.aoColumns[iDataIndex].bSortable === false )
    {
    return;
    }
    ...
    [/code]

    The important line of code here is the e.preventDefault(). With this line added, it seems to work correctly in both IE (Windows) and in Chrome (Mac).

    Since DataTables seems to be focusing on being Accessible, I'd say this is a pretty important fix. This may be a bug in IE or it could just be a "difference".
  • allanallan Posts: 54,880Questions: 1Answers: 8,601 Site admin
    I'm perfectly happy to add that in, but I'd like to understand why it is needed first. preventDefault will stop the browser taking the default action - which shouldn't be anything when clicking on a TH element!

    Did you keep the stopPropagation your own the event listener?

    Thanks,
    Allan
  • kdorffkdorff Posts: 16Questions: 0Answers: 0
    I am not familiar with "stopPropagation", so I doubt I did anything with it. You can see the entire code that demonstrates the problem in my jsfiddle above and you can see it happen if you can obtain access to a Windows machine or VM running Internet Explorer.

    The demo code binds to form submit event to demonstrate the submit is happening in IE. In my own app, I don't bind to submit(), I let submit do it's natural thing but it shouldn't be doing that when sorting headers. I do bind to the click events for some of the buttons, but when seems to be happening is when I sort with keyboard on the header is a form submit (and not necessarily a button click?).

    I concur that clicking a TH element shouldn't submit the form. Clicking it doesn't Clicking with the mouse behaves correctly. But my accessibility folks discovered that pressing return on the header, to sort with keyboard, is what submits the form. Why that should be, I don't know. I know that browsers often submit the form when you are on an input and press return and my only guess is that this is why IE is then submitting the form. I knew that with other buttons I wanted to click and not have submit I needed to add preventDefault() (as demonstrated in some jQuery UI demo I looked at) so I suspected adding that to the right place would fix it and I was correct. WHY exactly it would be needed, I don't know. Some oddity in IE, I guess.
  • dsimmonsdsimmons Posts: 7Questions: 0Answers: 0
    edited September 2013
    We are using 1.9.4 and IE8 and are seeing this exact same behavior, but not consistently. We attributed the difference to a page than contains multiple DataTables versus a page that contained one. The page with multiple tables failed accessibility. We ended up removing the sort capability on all the tables in order to pass the accessibility testing, but obviously, that removed the feature for those using a mouse instead of the enter key to sort.

    If I don't tab over a button prior to tabbing through column headers, the sort works fine; with a button it seems that the button somehow keeps focus and activates its functionality when the enter key is pressed. I will try the above fix.
  • isherwoodisherwood Posts: 10Questions: 0Answers: 0
    edited October 2013
    We're struggling with the same issue in IE including v10. Kdorff's solution works (thank you very much for that), but requires us to use the uncompressed file (365KB vs. 69KB).

    Of course I could re-compress, but it's still a core hack reducing maintainability. If anyone has found a way to implement this externally to the source file, please post up.

    Thanks, and thanks to Allan for a great extension.

    Update: I've done a compression on the modified file and it's 109KB. That'll do for now. Thanks again.
  • allanallan Posts: 54,880Questions: 1Answers: 8,601 Site admin
    Can anyone link me to a page which shows this issue? As I say, I'm very happy to include this, but I've not seen the issue yet myself - unless I'm just being blind!

    Allan
  • kdorffkdorff Posts: 16Questions: 0Answers: 0
    Hi Alan,

    Look at the top of this thread at my second post from Aug 10. I provided a jsfiddle demo of the problem. Run it in IE (notably, at least, in IE10 - current IE). Use your tab key to go to the header you want to sort. Press return. See it pop up the fact that the submit occured.

    Repeat this in any other browser and it doesn't popup but sort occurs as it should.

    I've found, and it seems others have verified, adding one line of code fixes the problem. I'd also love to to see a release version that contains this so I don't have to use my own modified version.

    Thanks,
    Kevin
  • dsimmonsdsimmons Posts: 7Questions: 0Answers: 0
    Found that if I define buttons as the sort works as expected. Refer to:

    http://stackoverflow.com/questions/12859190/ie8-delivering-focus-and-click-to-a-button-on-updating-a-text-field
  • kdorffkdorff Posts: 16Questions: 0Answers: 0
    Since the button is no longer a submit button, it doesn't surprise me that submit isn't executed.

    But, I still contend that unless you integrated my solution, that in IE DataTables is behaving incorrectly (in IE) when you try to sort using the keyboard (tab plus enter). Using tab + enter in any browser shouldn't submit the table, even if the tables DOES contain a submit button. In fact, I believe I should be able to have a submit button in my DataTable and people using IE and keyboard for navigation (for accessibility, such as the blind using screen readers, who commonly use IE in Windows) should still be able to use my table without it submitting when you sort.

    Is there a reason you believe my single line of additional code shouldn't be included in DataTables since it fixes using DataTables in IE and doesn't break DataTables for other browsers? I believe it does solve the problem and I cannot see that it would have negative side effects.
  • deepugndeepugn Posts: 4Questions: 0Answers: 0
    Hi Allan,

    I am also facing the same problem as kdorff, Have you taken any decisions on this?

    Best Regards,
    Deepu
  • temeteme Posts: 4Questions: 2Answers: 0
    Hi Allan,

    I run into the same issue in IE10 with 1.9.4.. I was able to get around by adding e.preventDefault() as Kevin suggested. I first tried to get around it by adding stopPropagation but that did not do it.
    I also run into the same issue with the previous/next links for pagination. Similar fix worked for those cases. I noticed I did not have an issue with the page index links in the "full_numbers" pagination option. Turns out the handler for the page index links already uses "e.preventDefault()" to avoid the issue. It is actually the only place preventDefault is used so it is easy to find the referance. I think it would be a good idea to put the fix in all the spots. We can narrow invocation to just cases where eventKey code is enter key (13). Here are the changes I made:


    Relevant change in call cases is:

    /* Avoid form submition when return key is pressed. */
    if (e.keyCode == 13)
    {
    e.preventDefault();
    }



    For Sort Header:


    function _fnSortAttachListener ( oSettings, nNode, iDataIndex, fnCallback )
    {
    _fnBindAction( nNode, {}, function (e) {
    /* Avoid form submition when return key is pressed. */
    if (e.keyCode == 13)
    {
    e.preventDefault();
    }

    For full_number pagination next/prev/first/last:

    "full_numbers": {
    /*
    * Function: oPagination.full_numbers.fnInit
    * Purpose: Initialise dom elements required for pagination with a list of the pages
    * Returns: -
    * Inputs: object:oSettings - dataTables settings object
    * node:nPaging - the DIV which contains this pagination control
    * function:fnCallbackDraw - draw function which must be called on update
    */
    "fnInit": function ( oSettings, nPaging, fnCallbackDraw )
    {
    var oLang = oSettings.oLanguage.oPaginate;
    var oClasses = oSettings.oClasses;
    var fnClickHandler = function ( e ) {
    if ( oSettings.oApi._fnPageChange( oSettings, e.data.action ) )
    {
    fnCallbackDraw( oSettings );
    }
    /* Avoid form submition when return key is pressed. */
    if (e.keyCode == 13)
    {
    e.preventDefault();
    }
    };



    For tow_button pagination next/prev:


    "two_button": {
    /*
    * Function: oPagination.two_button.fnInit
    * Purpose: Initialise dom elements required for pagination with forward/back buttons only
    * Returns: -
    * Inputs: object:oSettings - dataTables settings object
    * node:nPaging - the DIV which contains this pagination control
    * function:fnCallbackDraw - draw function which must be called on update
    */
    "fnInit": function ( oSettings, nPaging, fnCallbackDraw )
    {
    var oLang = oSettings.oLanguage.oPaginate;
    var oClasses = oSettings.oClasses;
    var fnClickHandler = function ( e ) {
    if ( oSettings.oApi._fnPageChange( oSettings, e.data.action ) )
    {
    fnCallbackDraw( oSettings );
    }
    /* Avoid form submition when return key is pressed. */
    if (e.keyCode == 13)
    {
    e.preventDefault();
    }
    };




    Here is the existing code in full_number pagination that already uses e.preventDefault() to avoid summit...



    /*
    * Function: oPagination.full_numbers.fnUpdate
    * Purpose: Update the list of page buttons shows
    * Returns: -
    * Inputs: object:oSettings - dataTables settings object
    * function:fnCallbackDraw - draw function to call on page change
    */
    "fnUpdate": function ( oSettings, fnCallbackDraw )
    {
    if ( !oSettings.aanFeatures.p )
    {
    return;
    }

    var iPageCount = DataTable.ext.oPagination.iFullNumbersShowPages;
    var iPageCountHalf = Math.floor(iPageCount / 2);
    var iPages = Math.ceil((oSettings.fnRecordsDisplay()) / oSettings._iDisplayLength);
    var iCurrentPage = Math.ceil(oSettings._iDisplayStart / oSettings._iDisplayLength) + 1;
    var sList = "";
    var iStartButton, iEndButton, i, iLen;
    var oClasses = oSettings.oClasses;
    var anButtons, anStatic, nPaginateList, nNode;
    var an = oSettings.aanFeatures.p;
    var fnBind = function (j) {
    oSettings.oApi._fnBindAction( this, {"page": j+iStartButton-1}, function(e) {
    /* Use the information in the element to jump to the required page */
    oSettings.oApi._fnPageChange( oSettings, e.data.page );
    fnCallbackDraw( oSettings );
    e.preventDefault();
    } );
    };



    Thanks,
    - Teme
  • allanallan Posts: 54,880Questions: 1Answers: 8,601 Site admin
    Can you try it with 1.10 please? I believe it should work there.

    Allan
  • kdorffkdorff Posts: 16Questions: 0Answers: 0
    Hi,

    I tried using 1.10.0-rc.2.dev as seen in

    http://jsfiddle.net/NPYPv/16/

    This version continues to work correctly in Chrome (as 1.9.x did), but this version still DOESN'T work in IE (tested with IE 11.0.9600.17041).

    Looking at the source for 1.1.0-rc.2.dev, it doesn't include the e.preventDefault() that fixed things for both teme and me. On first inspection, I would say teme's recommendations are more complete and isolated than mine and are likely what will be required.

    dsimmons, isherwood, teme and deepugn: can you test my latest jsfiddle (mentioned just above in this message) and see if 1.10-rc.2.dev fixes things in IE for you?

    Thanks,
    Kevin
  • allanallan Posts: 54,880Questions: 1Answers: 8,601 Site admin
    Apologies, this slipped my mind before the rc release. Yes, this does need to go in and I've just committed the change: https://github.com/DataTables/DataTablesSrc/commit/aaa08313f1 .

    The nightly is building now and will be available with the change shortly. Thanks for flagging this up!

    Allan
  • kdorffkdorff Posts: 16Questions: 0Answers: 0
    I'm a little surprised that you didn't take teme's modifications over mine, they appear to be more complete, but I have a strong feeling that mine fixes the exact problem I was having. Perhaps temes fixed a couple more problems. Either way, thanks.
  • allanallan Posts: 54,880Questions: 1Answers: 8,601 Site admin
    A lot of the suggestions above were relevant to DataTables prior to v1.10, which had a rewrite of the pagination control, and now uses the centralised event handler. Having said that, I did duff up and the change should have gone into that centralised event handler rather than where I put it.

    It is now: https://github.com/DataTables/DataTablesSrc/commit/c5e2f8c350

    Doh...!

    Allan
  • kdorffkdorff Posts: 16Questions: 0Answers: 0

    I updated my example based on the css/js from DataTables CDN for 1.10.0 as mentioned on the datatables.net home page.

    http://jsfiddle.net/NPYPv/19/

    I'm happy to report that 1.10.0 seems to work correctly (column sorting) out of the box in Windows/Internet Explorer (at least for my simple example) and Mac/Chrome.

    For the next release of my application, I will include 1.10.0 and make sure to have my accessibility team poke at it.

  • allanallan Posts: 54,880Questions: 1Answers: 8,601 Site admin

    That would be great - thanks. As I say, any feedback (good or bad) is welcome.

    Allan

This discussion has been closed.