Closed Bug 1263906 Opened 4 years ago Closed 4 years ago

When starting to edit a storage inspector field, it scrolls away from the view

Categories

(DevTools :: Storage Inspector, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: jsnajdr, Assigned: jsnajdr)

Details

Attachments

(1 file, 1 obsolete file)

Steps to reproduce:
1. Open Storage Inspector and the list of cookies or localStorage items.
2. The list needs to be long enough to make the table scrollable.
3. Double-click to edit some cell in the first row.

Expected result:
The cell starts to be edited, and is visible.

Actual result:
The cell scrolls away and is hidden under the table header.

This is caused by the table header having a "position: sticky" CSS style. Element.scrollIntoView() doesn't work correctly in this case.

The best solution seems to be to not call scrollIntoView at all: if the user managed to click on it, it means that it's visible, at least partially. The worst thing that can happen is that the cell is partially hidden.
Assignee: nobody → jsnajdr
Attachment #8740402 - Flags: review?(mratcliffe)
Comment on attachment 8740402 [details] [diff] [review]
When starting to edit a storage inspector field, it scrolls away from the view

Review of attachment 8740402 [details] [diff] [review]:
-----------------------------------------------------------------

This patch makes editing fields with the keyboard more difficult because we are no longer scroll the edited element into view. To see this problem shift tab until the table needs to scroll upwards and you are editing behind the headers again.

We get much better behavior if we prevent it aligning with the top of the table by setting alignToTop to false (default is true):
```
target.scrollIntoView(false);
```

We should also run this through try because it is exactly this kind of change that breaks tests.
Attachment #8740402 - Flags: review?(mratcliffe)
Added back scrollIntoView(false), scrolls to bottom instead to top. Behaves reasonably well both with mouse and keyboard. Still does unnecessary scrolling (to bottom), but is much better than the previous behavior that was hiding the edited field under the header.
Attachment #8742315 - Flags: review?(mratcliffe)
Attachment #8740402 - Attachment is obsolete: true
Attachment #8742315 - Flags: review?(mratcliffe) → review+
Try run is green, except unrelated intermittents in webconsole and debugger.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7dd5ac3bed0c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
I have reproduced this bug with Firefox Nightly 48.0a1 (Build ID:20160412030235) on 
Windows 8.1, 64-bit.

Verified as fixed with Firefox Release 48.0 (Build ID: 20160726073904)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
QA Whiteboard: [bugday-20160803]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.