Reps: ensure that grip arrays work even when a provider is not provided

RESOLVED FIXED in Firefox 50

Status

()

Firefox
Developer Tools: Shared Components
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: linclark, Assigned: Honza)

Tracking

Trunk
Firefox 50
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [devtools-html])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Currently, if you pass in [1, "foo", {}] to the grip array, the tiny mode will output `[3]` and the other modes will output `[]`.
(Reporter)

Updated

2 years ago
Whiteboard: [devtools-html] [triage]
(Reporter)

Updated

2 years ago
Blocks: 1282465

Updated

2 years ago
Blocks: 1263741

Updated

2 years ago
Flags: qe-verify-
Priority: -- → P2
Whiteboard: [devtools-html] [triage] → [devtools-html]
(Assignee)

Comment 1

2 years ago
Created attachment 8767128 [details] [diff] [review]
bug1282463.patch

Patch attached.

Lin, let me know if I should assign somebody else for the review.

Honza
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Attachment #8767128 - Flags: review?(lclark)

Updated

2 years ago
Iteration: --- → 50.2 - Jul 4
Priority: P2 → P1
(Reporter)

Comment 2

2 years ago
Comment on attachment 8767128 [details] [diff] [review]
bug1282463.patch

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

Manually tested this and it works for me. 

We talked about the additional review in the meeting. Unless I'm missing something, for this issue I don't think that we need to worry about testing all possible grip types. We may want to ask someone to review the rep system generally to make sure that we've covered all grip types though.
Attachment #8767128 - Flags: review?(lclark) → review+
(Assignee)

Comment 3

2 years ago
OK, great. I was actually retesting this again with bunch of different data and all works for me so, I totally agree with you.

Thanks Lin!
Honza
Flags: needinfo?(poirot.alex)
Keywords: checkin-needed
(Assignee)

Comment 4

2 years ago
Ah, removing unwanted NI, sorry.
Honza
Flags: needinfo?(poirot.alex)

Comment 5

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/668d29620e0b
Reps: ensure that grip arrays work even when a provider is not provided; r=linclark
Keywords: checkin-needed

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/668d29620e0b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.