next-/prev-unstarred shortcuts skip a failure after starring twice with onlyunstarred=1

RESOLVED FIXED

Status

Tree Management Graveyard
TBPL
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: emorley, Assigned: mbrubeck)

Tracking

Dependency tree / graph

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
1) Go to https://tbpl.mozilla.org/?tree=Mozilla-Inbound&onlyunstarred=1
2) Press 'k'
3) Star the first failure (click on the suggested bug star, then the quickstar button)
4) Press 'k'
5) Star this failure
6) Press 'k'

Expected:
After step #6, the first visible failure is selected.

Actual:
After step #6, the second visible failure is selected.

(Works fine at step #4, just broken for the second starring)

Must have not starred two in a row like this (without clicking), when I was testing bug 776692.
(Assignee)

Comment 1

6 years ago
It looks like this happens when you have onlyunstarred=1 and the selected job is hidden because it is starred.
Summary: next-/prev-unstarred shortcuts skip a failure after starring twice → next-/prev-unstarred shortcuts skip a failure after starring twice with onlyunstarred=1
(Assignee)

Comment 2

6 years ago
Created attachment 646193 [details] [diff] [review]
patch

The next/prev-unstarred shortcut handler sorts the results by "result.order".  This does not work correctly when the selected result is hidden, because _buildHTMLForOS sets .order only for displayed results.  (For hidden results it will be uninitialized or set to an old incorrect value.)

This patch fixes this by changing _buildHTMLForOS to set .order on *all* of the results, before filtering out the hidden ones.  (Note: The "order" property is currently used only for these keyboard shortcuts, so this shouldn't affect anything else.)
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Attachment #646193 - Flags: review?(bmo)
(Reporter)

Comment 3

6 years ago
Comment on attachment 646193 [details] [diff] [review]
patch

Looks good & works well locally for me :-)
Attachment #646193 - Flags: review?(bmo) → review+
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Updated

6 years ago
Depends on: 779658
Product: Webtools → Tree Management
Product: Tree Management → Tree Management Graveyard
You need to log in before you can comment on or make changes to this bug.