[BACKEND] Make localstorage entry rows editable via double-click in storage inspector

RESOLVED FIXED in Firefox 48

Status

()

Firefox
Developer Tools: Storage Inspector
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: miker, Assigned: miker)

Tracking

unspecified
Firefox 48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
Summary: Make localstorage entry rows editable via double-click in storage inspector → [BACKEND] Make localstorage entry rows editable via double-click in storage inspector
Created attachment 8732257 [details]
MozReview Request: Bug 1231155 - Make localstorage entry rows editable via double-click in storage inspector r=pbro

Review commit: https://reviewboard.mozilla.org/r/41059/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41059/
Attachment #8732257 - Flags: review?(pbrosset)
Comment on attachment 8732257 [details]
MozReview Request: Bug 1231155 - Make localstorage entry rows editable via double-click in storage inspector r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41059/diff/1-2/
Comment on attachment 8732257 [details]
MozReview Request: Bug 1231155 - Make localstorage entry rows editable via double-click in storage inspector r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41059/diff/2-3/

Updated

2 years ago
Blocks: 1231438
Assignee: nobody → mratcliffe
Blocks: 1260380
Comment on attachment 8732257 [details]
MozReview Request: Bug 1231155 - Make localstorage entry rows editable via double-click in storage inspector r=pbro

https://reviewboard.mozilla.org/r/41059/#review39439

The code changes are really simple, and apart from a couple of minor comments (below), I don't see any problem with them.
I just would like to note however that the columns are changing sizes sort of randomly when entering edit mode and tabbing through cells. We've discussed this before, but I just would like to bring it to your attention again because it's pretty anoying when editing storage values and we should look into fixing this pretty soon before the next release point.
Setting fixed dimensions on the columns would help (using % units of the table container maybe).

::: devtools/client/storage/test/storage-localstorage.html:4
(Diff revision 3)
> +<!doctype html>
> +<html>
> +  <!--
> +  Bug 970517 - Storage inspector front end - tests

I guess this comment should rather link to bug 1231155 instead.

::: devtools/server/actors/storage.js:1056
(Diff revision 3)
> +    editItem: method(Task.async(function*(data) {
> +      let {host, field, oldValue} = data;

Since you're already destructuring data, you might as well define `items` in there too, and maybe do this directly in the signature:

`function*({host, field, oldValue, items})`
Attachment #8732257 - Flags: review?(pbrosset) → review+
Quick comment, you should change the commit message from
Bug 1231155 - [BACKEND] Make localstorage entry rows editable via double-click in storage inspector r?pbro
to
Bug 1231155 - Make localstorage entry rows editable via double-click in storage inspector r=pbro
https://reviewboard.mozilla.org/r/41059/#review39439

> Since you're already destructuring data, you might as well define `items` in there too, and maybe do this directly in the signature:
> 
> `function*({host, field, oldValue, items})`

OMG: I can't believe I didn't do that.
Comment on attachment 8732257 [details]
MozReview Request: Bug 1231155 - Make localstorage entry rows editable via double-click in storage inspector r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41059/diff/3-4/
Attachment #8732257 - Attachment description: MozReview Request: Bug 1231155 - [BACKEND] Make localstorage entry rows editable via double-click in storage inspector r?pbro → MozReview Request: Bug 1231155 - Make localstorage entry rows editable via double-click in storage inspector r=pbro
Keywords: checkin-needed

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7a71674ba4dd
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.