ICEfaces
  1. ICEfaces
  2. ICE-11223

ace:dataTable - ace:checkboxButtons triggering row selection

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.2.BETA
    • Fix Version/s: 4.2, EE-3.3.0.GA_P05
    • Component/s: ACE-Components
    • Labels:
      None
    • Environment:
      All

      Description

      An ace:dataTable is set with an ace:checkboxButtons component in one of its columns. Depending on the browser, clicking on these buttons is causing issues with the row selection of the dataTable.

      Chrome: Checking a checkbox causes the row to become selected/unselected and is also causing the check to be cleared.

      IE11/Firefox: Clicking on a checkbox button isn't causing row selection/unselection but if a row is selected/unselected it is unchecking a checked checkbox.

        Activity

        Arran Mccullough created issue -
        Hide
        Arran Mccullough added a comment -

        Attached test case.

        Steps:

        • Load welcomeICEfaces.jsf
        • In Chrome, click on a checkbox button. The row will be selected and the check will be cleared.
        • In Firefox/IE11, click on a checkbox button. It will be checked. Now select the row, the check will now be cleared.
        Show
        Arran Mccullough added a comment - Attached test case. Steps: Load welcomeICEfaces.jsf In Chrome, click on a checkbox button. The row will be selected and the check will be cleared. In Firefox/IE11, click on a checkbox button. It will be checked. Now select the row, the check will now be cleared.
        Arran Mccullough made changes -
        Field Original Value New Value
        Attachment Case14014Example.war [ 22429 ]
        Attachment Case14014Example.zip [ 22430 ]
        Ken Fyten made changes -
        Assignee Carmen Cristurean [ ccristurean ]
        Fix Version/s 4.2 [ 12870 ]
        Assignee Priority P1 [ 10010 ]
        Hide
        Carmen Cristurean added a comment - - edited

        Added similar test cases with ACE Input and Button components.

        Passing tests:
        Input Components (ace:textEntry, dateTimeEntry, textAreaEntry, maskedEntry, autoCompleteEntry, sliderEntry, colorEntry): /ICE-11223b.jsf - test appears to pass in all browsers (Chrome, MsEdge, FF49, IE11): the row selection does not change after entering/selecting text and pressing TAB/Enter.
        Note, ace:autoCompleteEntry passes only if selecting autoComplete options using the keyboard.

        Issues found:

        1) ace:pushButton: /ICE-11223a.jsf
        Test passes on Firefox49 and IE11, and it fails on MsEdge38, Chrome55 and Safari: pressing the pushButton changes the selected state on the row.

        2) ace:richtextEntry: /ICE-11223c.jsf - this test fails, the rows become selected after saving text in the ace:richTextEntry.
        Also, in Chrome, a JS error occurs when clicking the "Save" icon:

        ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:340 Uncaught TypeError: Cannot read property 'on' of undefined
        at $.attachListener (ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:340)
        at $.setup (ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:351)
        at $.m (ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:829)
        at ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:28
        attachListener @ ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:340
        setup @ ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:351
        m @ ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:829
        (anonymous) @ ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:28

        3) ace:radioButtons: /ICE-11223.jsf
        Test fails in the same manner as ace:checkboxButtons:

        • in Chrome/MsEdge, when clicking a button the row will be selected and the check cleared.
        • in FF/IE11, when clicking the button, it will be checked; when selecting a row, the check will be cleared.

        4) ace:menuButton: /ICE-11223a.jsf
        Test fails in all browsers, selecting a new menuItem changes the row selection.
        In MsEdge/Chrome, clicking the menuButton also changes the row selection, and the menuButton fails to open up and render its menuItems.

        5) ace:autoCompleteEntry /ICE-11223b.jsf
        Test fails in all browsers if selecting the autoComplete options with the mouse, the row selection changes in this case; the row selection does not change if using only the keyboard keys.

        6) ace:buttonGroup: /ICE-11223a.jsf
        Test fails only in Chrome/MsEdge: selecting a radioButton or checkboxButton inside the buttonGroup changes the selection state of the row.

        Show
        Carmen Cristurean added a comment - - edited Added similar test cases with ACE Input and Button components. Passing tests: Input Components (ace:textEntry, dateTimeEntry, textAreaEntry, maskedEntry, autoCompleteEntry, sliderEntry, colorEntry): / ICE-11223 b.jsf - test appears to pass in all browsers (Chrome, MsEdge, FF49, IE11): the row selection does not change after entering/selecting text and pressing TAB/Enter. Note, ace:autoCompleteEntry passes only if selecting autoComplete options using the keyboard. Issues found: 1) ace:pushButton: / ICE-11223 a.jsf Test passes on Firefox49 and IE11, and it fails on MsEdge38, Chrome55 and Safari: pressing the pushButton changes the selected state on the row. 2) ace:richtextEntry: / ICE-11223 c.jsf - this test fails, the rows become selected after saving text in the ace:richTextEntry. Also, in Chrome, a JS error occurs when clicking the "Save" icon: ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:340 Uncaught TypeError: Cannot read property 'on' of undefined at $.attachListener (ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:340) at $.setup (ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:351) at $.m (ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:829) at ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:28 attachListener @ ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:340 setup @ ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:351 m @ ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:829 (anonymous) @ ckeditor.js.jsf?ln=icefaces.ace&v=4_2_0_170126:28 3) ace:radioButtons: / ICE-11223 .jsf Test fails in the same manner as ace:checkboxButtons: in Chrome/MsEdge, when clicking a button the row will be selected and the check cleared. in FF/IE11, when clicking the button, it will be checked; when selecting a row, the check will be cleared. 4) ace:menuButton: / ICE-11223 a.jsf Test fails in all browsers, selecting a new menuItem changes the row selection. In MsEdge/Chrome, clicking the menuButton also changes the row selection, and the menuButton fails to open up and render its menuItems. 5) ace:autoCompleteEntry / ICE-11223 b.jsf Test fails in all browsers if selecting the autoComplete options with the mouse, the row selection changes in this case; the row selection does not change if using only the keyboard keys. 6) ace:buttonGroup: / ICE-11223 a.jsf Test fails only in Chrome/MsEdge: selecting a radioButton or checkboxButton inside the buttonGroup changes the selection state of the row.
        Ken Fyten made changes -
        Assignee Carmen Cristurean [ ccristurean ] Mircea Toma [ mircea.toma ]
        Fix Version/s EE-4.2.0.GA [ 13071 ]
        Fix Version/s 4.2 [ 12870 ]
        Assignee Priority P1 [ 10010 ] P2 [ 10011 ]
        Hide
        Mircea Toma added a comment -

        Fix filtering of elements that can trigger a row selection. This blocks 'click' events triggered by components located in the data table cells to select/deselect a row.

        Show
        Mircea Toma added a comment - Fix filtering of elements that can trigger a row selection. This blocks 'click' events triggered by components located in the data table cells to select/deselect a row.
        Mircea Toma made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #50351 Wed Feb 01 12:00:06 MST 2017 mircea.toma ICE-11223 Fix filtering of elements that can trigger a row selection.
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
        Hide
        Carmen Cristurean added a comment - - edited

        Tested with ICEfaces4 trunk r50353 in MsEdge38, Chrome55, FF49, IE11:
        Issues 1) to 6) above have been resolved.

        On /ICE-11223.jsf: when clicking a checkbox button/ radio button, it will be checked, but selecting the row will clear the button selection.
        This behavior is now in all browsers, is it expected?

        Show
        Carmen Cristurean added a comment - - edited Tested with ICEfaces4 trunk r50353 in MsEdge38, Chrome55, FF49, IE11: Issues 1) to 6) above have been resolved. On / ICE-11223 .jsf: when clicking a checkbox button/ radio button, it will be checked, but selecting the row will clear the button selection. This behavior is now in all browsers, is it expected?
        Hide
        Mircea Toma added a comment - - edited

        Q: This behavior is now in all browsers, is it expected?
        A: No, it's not. It seems that this is not a component issue. I modified ICE-11223.jsf page to have the checkboxes and radio buttons AJAX submit their state changes thus any incoming updates will not change the state on the client.

        Show
        Mircea Toma added a comment - - edited Q: This behavior is now in all browsers, is it expected? A: No, it's not. It seems that this is not a component issue. I modified ICE-11223 .jsf page to have the checkboxes and radio buttons AJAX submit their state changes thus any incoming updates will not change the state on the client.
        Carmen Cristurean made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Ken Fyten made changes -
        Assignee Priority P2 [ 10011 ] P1 [ 10010 ]
        Hide
        Ken Fyten added a comment -

        This fix needs to target focussable input elements, not all elements.

        Show
        Ken Fyten added a comment - This fix needs to target focussable input elements, not all elements.
        Ken Fyten made changes -
        Fix Version/s 4.2 [ 12870 ]
        Fix Version/s EE-4.2.0.GA [ 13071 ]
        Hide
        Mircea Toma added a comment -

        Run double/click callbacks whenever the data table cell is double/clicked or any of it children. Previously only double/clicking the cell or the first child of it would have run the callbacks.

        Show
        Mircea Toma added a comment - Run double/click callbacks whenever the data table cell is double/clicked or any of it children. Previously only double/clicking the cell or the first child of it would have run the callbacks.
        Mircea Toma made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #50714 Wed Feb 22 16:13:34 MST 2017 mircea.toma ICE-11223 Run double/click callbacks whenever the data table cell is double/clicked or any of it children.
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
        Hide
        Mircea Toma added a comment -

        Exclude child input elements from triggering a cell edit or row selection.

        Show
        Mircea Toma added a comment - Exclude child input elements from triggering a cell edit or row selection.
        Hide
        Ken Fyten added a comment -

        Re-opened to block input comps from activating row selection.

        Show
        Ken Fyten added a comment - Re-opened to block input comps from activating row selection.
        Ken Fyten made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #50715 Wed Feb 22 16:35:41 MST 2017 mircea.toma ICE-11223 Exclude child input elements from triggering an cell edit or row selection.
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
        Hide
        Mircea Toma added a comment -

        Input elements are now blocked from activating row selection.

        Show
        Mircea Toma added a comment - Input elements are now blocked from activating row selection.
        Mircea Toma made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Ken Fyten made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Mircea Toma added a comment -

        I don't see the original issue reappearing. I used both the attached test case and dataTable/ICE-11223.jsf, they work as expected.

        Show
        Mircea Toma added a comment - I don't see the original issue reappearing. I used both the attached test case and dataTable/ ICE-11223 .jsf , they work as expected.
        Hide
        Mircea Toma added a comment -

        Modified datatable.js to have the 'click' and 'dblclick' events blocked from reaching the data table cell when they bubble through an input elements.

        Show
        Mircea Toma added a comment - Modified datatable.js to have the 'click' and 'dblclick' events blocked from reaching the data table cell when they bubble through an input elements.
        Mircea Toma made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #50755 Mon Feb 27 14:15:16 MST 2017 mircea.toma ICE-11223 'click' and 'dblclick' events are now blocked from reaching the table cell when they bubble through an input elements.
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
        Carmen Cristurean made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Mircea Toma added a comment -

        Fixed linkButton and menuButton issue by adding the anchor element to the list of element types that block event bubbling.

        Show
        Mircea Toma added a comment - Fixed linkButton and menuButton issue by adding the anchor element to the list of element types that block event bubbling.
        Hide
        Mircea Toma added a comment -

        I cannot reproduce the issue with richTextEntry. I used Chrome for testing.

        Show
        Mircea Toma added a comment - I cannot reproduce the issue with richTextEntry. I used Chrome for testing.
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #50765 Mon Feb 27 18:06:52 MST 2017 mircea.toma ICE-11223 Added the anchor element to the list of element types that block event bubbling.
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
        Hide
        Mircea Toma added a comment -

        Fixed autoCompleteEntry by wrapping the matching results in an anchor tag (replacing a DIV) so that the dataTable cell will detect itand block the event bubbling.

        Show
        Mircea Toma added a comment - Fixed autoCompleteEntry by wrapping the matching results in an anchor tag (replacing a DIV) so that the dataTable cell will detect itand block the event bubbling.
        Hide
        Mircea Toma added a comment - - edited

        I cannot reproduce the 'Missing resources for "ColorEntryInit" component.' JS error.

        Show
        Mircea Toma added a comment - - edited I cannot reproduce the 'Missing resources for "ColorEntryInit" component.' JS error.
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #50769 Tue Feb 28 12:50:46 MST 2017 mircea.toma ICE-11223 Wrap the matching results in an anchor tag (replacing a DIV) so that the dataTable cell will detect itand block the event bubbling.
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/src/org/icefaces/ace/component/autocompleteentry/AutoCompleteEntryRenderer.java
        Hide
        Mircea Toma added a comment -

        Removed rogue comma to allow for colorentry.js compression to complete successfully.

        Show
        Mircea Toma added a comment - Removed rogue comma to allow for colorentry.js compression to complete successfully.
        Mircea Toma made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #50773 Wed Mar 01 15:15:47 MST 2017 mircea.toma ICE-11223 Removed rogue comma to allow for colorentry.js compression to complete successfully.
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/colorentry/colorentry.js
        Hide
        Ken Fyten added a comment -

        The colorEntry issues above are resolved at r. 50772, however the test fails, as the row gets selected/unselected when clicking on the colorEntry outside of the color selection rectangle (all browsers).

        So long a the user clicks on an focusable element inside the color selection pane it works as expected, however, it is possible to trigger the row selector if clicking in side the pane in a whitespace area. This is a side-effect of how this must be implemented and isn't particularly harmful, so we'll consider it an acceptable known issue.

        It might be possible in the future to have the dataTable logic also check for the presence of a marker css class on an element which would tell it to ignore clicks from inside the element. This would provide a mechanism to avoid this.

        Show
        Ken Fyten added a comment - The colorEntry issues above are resolved at r. 50772, however the test fails, as the row gets selected/unselected when clicking on the colorEntry outside of the color selection rectangle (all browsers). So long a the user clicks on an focusable element inside the color selection pane it works as expected, however, it is possible to trigger the row selector if clicking in side the pane in a whitespace area. This is a side-effect of how this must be implemented and isn't particularly harmful, so we'll consider it an acceptable known issue. It might be possible in the future to have the dataTable logic also check for the presence of a marker css class on an element which would tell it to ignore clicks from inside the element. This would provide a mechanism to avoid this.
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #50924 Thu Mar 02 17:24:29 MST 2017 mircea.toma ICE-11223 Modify isBlurEvent function in blockui.js to give up traversing the call stack when it is not possible to determine the function caller.
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/core/src/main/javascript/blockui.js
        Ken Fyten made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Judy Guglielmin added a comment -

        reopened for Support Case 14185 to backport fix to ICEfaces 3.3.0 maintenance branch

        Show
        Judy Guglielmin added a comment - reopened for Support Case 14185 to backport fix to ICEfaces 3.3.0 maintenance branch
        Judy Guglielmin made changes -
        Resolution Fixed [ 1 ]
        Status Closed [ 6 ] Reopened [ 4 ]
        Judy Guglielmin made changes -
        Assignee Mircea Toma [ mircea.toma ] Arturo Zambrano [ artzambrano ]
        Judy Guglielmin made changes -
        Judy Guglielmin made changes -
        Fix Version/s EE-3.3.0.GA_P05 [ 13082 ]
        Judy Guglielmin made changes -
        Attachment SC14185.war [ 22594 ]
        Attachment SC14185.zip [ 22595 ]
        Hide
        Arturo Zambrano added a comment - - edited

        r51869: backported all fixes to the 3.3 EE maintenance branch

        Testing Notes: The last test case added (the one using ace:checkboxButton instead of ace:checkboxButtons) needs to be added <ace:ajax /> to the ace:checkboxButton, in order to save the checked state, otherwise, it's just a client-side component and the state is lost after selecting a row.

        Please also run all ace:autoCompleteEntry tests, since there was a modification for this fix to work.

        Show
        Arturo Zambrano added a comment - - edited r51869: backported all fixes to the 3.3 EE maintenance branch Testing Notes: The last test case added (the one using ace:checkboxButton instead of ace:checkboxButtons) needs to be added <ace:ajax /> to the ace:checkboxButton, in order to save the checked state, otherwise, it's just a client-side component and the state is lost after selecting a row. Please also run all ace:autoCompleteEntry tests, since there was a modification for this fix to work.
        Hide
        Arturo Zambrano added a comment - - edited

        It's important to mention that for this test case to work, the ace:checkboxButton needs to have it's value bound to a bean property and to have an <ace:ajax /> tag. Perhaps other components might need those two things to work as well.

        I had modified the original test case to use ace:checkboxButton as well, and I spent some time testing the backport either without ajax or without having the value bound to a bean property, and it wasn't working as expected. The first issue was solved (checking the ace:checkboxButton was being done successfully and without selecting the row it was in), but an issue remained where after checking an ace:checkboxButton and then selecting the row, the checked state was lost. This scenario only works if the ace:checkboxButton is using <ace:ajax /> and its value is bound to a bean property.

        Show
        Arturo Zambrano added a comment - - edited It's important to mention that for this test case to work, the ace:checkboxButton needs to have it's value bound to a bean property and to have an <ace:ajax /> tag. Perhaps other components might need those two things to work as well. I had modified the original test case to use ace:checkboxButton as well, and I spent some time testing the backport either without ajax or without having the value bound to a bean property, and it wasn't working as expected. The first issue was solved (checking the ace:checkboxButton was being done successfully and without selecting the row it was in), but an issue remained where after checking an ace:checkboxButton and then selecting the row, the checked state was lost. This scenario only works if the ace:checkboxButton is using <ace:ajax /> and its value is bound to a bean property.
        Hide
        Liana Munroe added a comment - - edited

        Verified using attached test case and ICEfaces EE-3.3.0_P05 Jenkins build 2, Tomcat 8, MS Edge, IE 11, 10, 9, FF 53, Chrome 60.

        When testing on IE 8 and 7, using Tomcat 8 or 7, the following console error was seen:
        IE 8 - SCRIPT5007: Unable to get property 'instanceTag' of undefined or null reference
        icepush.uncompressed.js (118,9). This issue is related to http://jira.icesoft.org/browse/ICE-11355. After removing the icepush-ee.jar the issue console error was no longer seen in IE 7 and 8.

        Show
        Liana Munroe added a comment - - edited Verified using attached test case and ICEfaces EE-3.3.0_P05 Jenkins build 2, Tomcat 8, MS Edge, IE 11, 10, 9, FF 53, Chrome 60. When testing on IE 8 and 7, using Tomcat 8 or 7, the following console error was seen: IE 8 - SCRIPT5007: Unable to get property 'instanceTag' of undefined or null reference icepush.uncompressed.js (118,9). This issue is related to http://jira.icesoft.org/browse/ICE-11355 . After removing the icepush-ee.jar the issue console error was no longer seen in IE 7 and 8.
        Hide
        Arturo Zambrano added a comment - - edited

        That seems to be the same issue as http://jira.icesoft.org/browse/ICE-11355.

        It doesn't happen when icepush.jar is not present.

        Show
        Arturo Zambrano added a comment - - edited That seems to be the same issue as http://jira.icesoft.org/browse/ICE-11355 . It doesn't happen when icepush.jar is not present.
        Ken Fyten made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Ken Fyten made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Arturo Zambrano
            Reporter:
            Arran Mccullough
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: