Clicking the "inspect" icon of the grid element in the Layout panel throws

RESOLVED FIXED in Firefox 61

Status

defect
RESOLVED FIXED
Last year
Last year

People

(Reporter: nchevobbe, Assigned: gl)

Tracking

Trunk
Firefox 61

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment)

**Steps to reproduce**
1. Go to data:text/html,<meta charset=utf8><p style="display: grid;">Hello</p>
2. Open the inspector
3. Select the Layout panel
4. Click on the icon next to the `p` element
5. Open the browser console


**Expected results**

There's no error thrown

**Actual results**

An error appears in the browser console: 

TypeError: setSelectedNode(...) is undefined - GridItem.js:88:5

---

The error is thrown from [1], because we try to call `.catch` on the result of `setSelectedNode`.
But `setSelectedNode` in the end calls `setNodeFront` (see [2]), which does not return anything.

The fix would be to remove the `catch` call and wrap setSelectedNode in a try/catch block instead.

[1] https://searchfox.org/mozilla-central/rev/11a2ae294f50049e12515b5821f5a396d951aacb/devtools/client/inspector/grids/components/GridItem.js#88
[2] https://searchfox.org/mozilla-central/rev/11a2ae294f50049e12515b5821f5a396d951aacb/devtools/client/framework/selection.js#134-146
[3] https://searchfox.org/mozilla-central/rev/11a2ae294f50049e12515b5821f5a396d951aacb/devtools/client/inspector/grids/components/GridItem.js#89
Assignee

Updated

Last year
Assignee: nobody → gl
Status: NEW → ASSIGNED
Comment on attachment 8971093 [details]
Bug 1455641 - Remove unnecessary catch from setSelectedNode and scrollIntoView calls.

https://reviewboard.mozilla.org/r/239884/#review245722

I agree on removing the `.catch()` part on `setSelectedNode` because that doesn't return a promise, so it's just wrong.
However `.scrollIntoView()` is a method call that does return a promise. So ideally we do want to catch errors in that second case. So I'd suggest keeping that part in.
Attachment #8971093 - Flags: review?(pbrosset) → review+

Comment 3

Last year
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aaba7b12e27b
Remove unnecessary catch from setSelectedNode calls. r=pbro
https://hg.mozilla.org/mozilla-central/rev/aaba7b12e27b
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61

Updated

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