You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by sr...@apache.org on 2020/11/03 14:52:59 UTC

[spark] branch master updated: [SPARK-33284][WEB-UI] In the Storage UI page, clicking any field to sort the table will cause the header content to be lost

This is an automated email from the ASF dual-hosted git repository.

srowen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 56c623e  [SPARK-33284][WEB-UI] In the Storage UI page, clicking any field to sort the table will cause the header content to be lost
56c623e is described below

commit 56c623e98c54fdb4d47c9264ae1b282ecb2b7291
Author: neko <ec...@gmail.com>
AuthorDate: Tue Nov 3 08:49:52 2020 -0600

    [SPARK-33284][WEB-UI] In the Storage UI page, clicking any field to sort the table will cause the header content to be lost
    
    ### What changes were proposed in this pull request?
    In the old version of spark in the storage UI page, the sorting function is normal, but sorting in the new version will cause the header content to be lost, So I try to fix the bug.
    
    ### Why are the changes needed?
    
    The header field of the table on the page is similar to the following, **note that each th contains the span attribute**:
    
    ```html
    <thead>
        <tr>
            ....
            <th width="" class="">
                  <span data-toggle="tooltip" title="" data-original-title="StorageLevel displays where the persisted RDD is stored, format of the persisted RDD (serialized or de-serialized) andreplication factor of the persisted RDD">
                    Storage Level
                  </span>
            </th>
           .....
        </tr>
    </thead>
    ```
    
    Since  [PR#26136](https://github.com/apache/spark/pull/26136), if the `th` in the table itself contains the `span` attribute, the `span` will be deleted directly after clicking the sort, and the original header content will be lost.
    
    There are three problems  in `sorttable.js`:
    
    1. `sortrevind.class = "sorttable_sortrevind"` in  [sorttab.js#107](https://github.com/apache/spark/blob/9d5e48ea95d1c3017a51ff69584f32a18901b2b5/core/src/main/resources/org/apache/spark/ui/static/sorttable.js#L107) and  `sortfwdind.class = "sorttable_sortfwdind"` in  [sorttab.js#125](https://github.com/apache/spark/blob/9d5e48ea95d1c3017a51ff69584f32a18901b2b5/core/src/main/resources/org/apache/spark/ui/static/sorttable.js#L125)
    sorttable_xx attribute should be assigned to`className` instead of `class`, as javascript uses `rowlists[j].className.search` rather than `rowlists[j].class.search` to determine whether the component has a sorting flag or not.
    2.  `rowlists[j].className.search(/\sorttable_sortrevind\b/)` in  [sorttab.js#120](https://github.com/apache/spark/blob/9d5e48ea95d1c3017a51ff69584f32a18901b2b5/core/src/main/resources/org/apache/spark/ui/static/sorttable.js#L120) was wrong. The original intention is to search whether `className` contains  the word `sorttable_sortrevind` , but the expression is wrong,  it should be `\bsorttable_sortrevind\b` instead of `\sorttable_sortrevind\b`
    3. The if check statement in the following code snippet ([sorttab.js#141](https://github.com/apache/spark/blob/9d5e48ea95d1c3017a51ff69584f32a18901b2b5/core/src/main/resources/org/apache/spark/ui/static/sorttable.js#L141)) was wrong. **If the `search` function does not find the target, it will return -1, but Boolean(-1) is actually equals true**. This statement will cause span to be deleted even if it does not contain `sorttable_sortfwdind` or `sorttable_sortrevind`.
    ```javascript
    rowlists = this.parentNode.getElementsByTagName("span");
    for (var j=0; j < rowlists.length; j++) {
                  if (rowlists[j].className.search(/\bsorttable_sortfwdind\b/)
                      || rowlists[j].className.search(/\sorttable_sortrevind\b/) ) {
                      rowlists[j].parentNode.removeChild(rowlists[j]);
                  }
              }
    ```
    
    ### Does this PR introduce _any_ user-facing change?
    NO.
    
    ### How was this patch tested?
    The manual test result of the ui page is as below:
    
    ![fix sorted](https://user-images.githubusercontent.com/52202080/97543194-daeaa680-1a02-11eb-8b11-8109c3e4e9a3.gif)
    
    Closes #30182 from akiyamaneko/ui_storage_sort_error.
    
    Authored-by: neko <ec...@gmail.com>
    Signed-off-by: Sean Owen <sr...@gmail.com>
---
 .../main/resources/org/apache/spark/ui/static/sorttable.js | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/core/src/main/resources/org/apache/spark/ui/static/sorttable.js b/core/src/main/resources/org/apache/spark/ui/static/sorttable.js
index ecd580e..3f98a03 100644
--- a/core/src/main/resources/org/apache/spark/ui/static/sorttable.js
+++ b/core/src/main/resources/org/apache/spark/ui/static/sorttable.js
@@ -99,12 +99,12 @@ sorttable = {
                                                     'sorttable_sorted_reverse');
             rowlists = this.parentNode.getElementsByTagName("span");
             for (var j=0; j < rowlists.length; j++) {
-                if (rowlists[j].className.search(/\bsorttable_sortfwdind\b/)) {
+                if (rowlists[j].className.search(/\bsorttable_sortfwdind\b/) != -1) {
                     rowlists[j].parentNode.removeChild(rowlists[j]);
                 }
             }
             sortrevind = document.createElement('span');
-            sortrevind.class = "sorttable_sortrevind";
+            sortrevind.className = "sorttable_sortrevind";
             sortrevind.innerHTML = stIsIE ? '&nbsp<font face="webdings">5</font>' : '&nbsp;&#x25BE;';
             this.appendChild(sortrevind);
             return;
@@ -117,12 +117,12 @@ sorttable = {
                                                   'sorttable_sorted');
             rowlists = this.parentNode.getElementsByTagName("span");
             for (var j=0; j < rowlists.length; j++) {
-                if (rowlists[j].className.search(/\sorttable_sortrevind\b/)) {
+                if (rowlists[j].className.search(/\bsorttable_sortrevind\b/) != -1) {
                     rowlists[j].parentNode.removeChild(rowlists[j]);
                 }
             }
             sortfwdind = document.createElement('span');
-            sortfwdind.class = "sorttable_sortfwdind";
+            sortfwdind.className = "sorttable_sortfwdind";
             sortfwdind.innerHTML = stIsIE ? '&nbsp<font face="webdings">6</font>' : '&nbsp;&#x25B4;';
             this.appendChild(sortfwdind);
             return;
@@ -138,15 +138,15 @@ sorttable = {
           });
           rowlists = this.parentNode.getElementsByTagName("span");
           for (var j=0; j < rowlists.length; j++) {
-              if (rowlists[j].className.search(/\bsorttable_sortfwdind\b/)
-                  || rowlists[j].className.search(/\sorttable_sortrevind\b/) ) {
+              if (rowlists[j].className.search(/\bsorttable_sortfwdind\b/) != -1
+                  || rowlists[j].className.search(/\bsorttable_sortrevind\b/) != -1) {
                   rowlists[j].parentNode.removeChild(rowlists[j]);
               }
           }
 
           this.className += ' sorttable_sorted';
           sortfwdind = document.createElement('span');
-          sortfwdind.class = "sorttable_sortfwdind";
+          sortfwdind.className = "sorttable_sortfwdind";
           sortfwdind.innerHTML = stIsIE ? '&nbsp<font face="webdings">6</font>' : '&nbsp;&#x25B4;';
           this.appendChild(sortfwdind);
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org