Closed Bug 1273584 Opened 9 years ago Closed 9 years ago

Displayed device list pref grows forever

Categories

(DevTools :: Responsive Design Mode, defect, P1)

Product:
DevTools
Component:
Responsive Design Mode
Type:
defect

Tracking

(firefox49 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 49
Iteration:
49.3 - Jun 6
Tracking Flags:
Tracking Status
firefox49 --- fixed

People

(Reporter: jryans, Assigned: jryans)

References

Details

(Whiteboard: [multiviewport] [mvp-rdm])

Attachments

(1 file)

MozReview Request: Bug 1273584 - Stop growing device list forever. r=gl
(deleted), text/x-review-board-request
gl
: review+
Details
I noticed recently that we are always appending the default displayed devices to the array, whether or not they are in there. This means the array is always growing in size, with more and more duplicates.
This version should work, but we may actually want a data structure that deduplicates. At the moment, those of us who work on the tool would still have large pref data even with this change. https://treeherder.mozilla.org/#/jobs?repo=try&revision=8769e03479ab
https://reviewboard.mozilla.org/r/53268/#review50052 ::: devtools/client/responsive.html/devices.js:37 (Diff revision 1) > - if (newDevice.displayed) { > + if (newDevice.displayed && !deviceList.includes(newDevice.name)) { > deviceList.push(newDevice.name); Switch this to a JavaScript Set and you'll get your deduplicating and won't have to add this condition. You can then use Array.from(deviceSet) to get an array out of it for any external APIs.
https://reviewboard.mozilla.org/r/53268/#review50052 > Switch this to a JavaScript Set and you'll get your deduplicating and won't have to add this condition. > > You can then use Array.from(deviceSet) to get an array out of it for any external APIs. Yep, a Set had ocurred to me too! I couldn't remember if you can iterate in insertion order, but it looks like you can. I'll give it a try.
Comment on attachment 8753434 [details] MozReview Request: Bug 1273584 - Stop growing device list forever. r=gl Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53268/diff/1-2/
Attachment #8753434 - Flags: review?(gl)
Comment on attachment 8753434 [details] MozReview Request: Bug 1273584 - Stop growing device list forever. r=gl https://reviewboard.mozilla.org/r/53268/#review50564
Attachment #8753434 - Flags: review?(gl) → review+
Iteration: --- → 49.2 - May 23
Flags: qe-verify-
Priority: -- → P1
Whiteboard: [multiviewport][triage] → [multiviewport] [mvp-rdm]
Seems to be fine in my WinXP try run above, let's try landing again.
Iteration: 49.2 - May 23 → 49.3 - Jun 6
https://hg.mozilla.org/mozilla-central/rev/3aa793fa2d0b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: