ODIn/spark-instrumented-optimizer
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

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

Browse source 
### 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](9d5e48ea95/core/src/main/resources/org/apache/spark/ui/static/sorttable.js (L107)) and  `sortfwdind.class = "sorttable_sortfwdind"` in  [sorttab.js#125](9d5e48ea95/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](9d5e48ea95/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](9d5e48ea95/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 <echohlne@gmail.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
This commit is contained in:
neko 2020-11-03 08:49:52 -06:00 committed by Sean Owen
parent 4c8ee8856c
commit 56c623e98c
1 changed files with 7 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
core/src/main/resources/org/apache/spark/ui/static/sorttable.js
View file

@ -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);