Reps: Grips rep renders only 3 properties in long mode

RESOLVED FIXED in Firefox 50

Status

P1
normal
RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: Honza, Assigned: Honza)

Tracking

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

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [devtools-html])

Attachments

(1 attachment, 2 obsolete attachments)

Follow up for bug 1264685.

Grip rep should render 100 (max) properties in the long mode. It's rendering just 3. The longIterator should be used in Grip.render method in case this.props.mode == "long"

Honza

Updated

2 years ago
Blocks: 1263741
Whiteboard: [devtools-html] [triage]

Updated

2 years ago
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Iteration: --- → 50.2
Flags: qe-verify-
Priority: -- → P1
Whiteboard: [devtools-html] [triage] → [devtools-html]
Depends on: 1264685
Created attachment 8764611 [details] [diff] [review]
bug1281489.patch

Long mode in the grip + the grip test fixed.

I am not sure how to text max number of props for the long mode. The grip stub could be pretty huge (> 100 props). Should we generate the `ownProperties` field?

Honza
Attachment #8764611 - Flags: review?(lclark)
Comment on attachment 8764611 [details] [diff] [review]
bug1281489.patch

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

So far this looks good to me. 

I think generating the ownProperties field makes sense. Clearing the review for now, pending the test update.
Attachment #8764611 - Flags: review?(lclark)
Created attachment 8764887 [details] [diff] [review]
bug1281489.patch

(In reply to Lin Clark [:linclark] from comment #2)
> I think generating the ownProperties field makes sense.
Done

Honza
Attachment #8764611 - Attachment is obsolete: true
Attachment #8764887 - Flags: review?(lclark)
Comment on attachment 8764887 [details] [diff] [review]
bug1281489.patch

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

Other than one minor issue, this looks good to me!

::: devtools/client/shared/components/test/mochitest/test_reps_grip.html
@@ +280,1 @@
>              "ownPropertiesLength": 5,

This should be changed to 101.
Attachment #8764887 - Flags: review?(lclark) → review+
Created attachment 8765422 [details] [diff] [review]
bug1281489.patch

(In reply to Lin Clark [:linclark] from comment #4)
> This should be changed to 101.
Good catch, fixed!

Honza
Attachment #8764887 - Attachment is obsolete: true
Attachment #8765422 - Flags: review+
Keywords: checkin-needed

Comment 6

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/c0fb7d447b9d
Fix Grip rep long mode. r=linclark
Keywords: checkin-needed

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c0fb7d447b9d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50

Updated

5 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.