Closed Bug 1578745 Opened 11 months ago Closed 9 months ago

Introduce a preference to enable fission frame inspection in DevTools

Categories

(DevTools :: Framework, enhancement, P1)

enhancement

Tracking

(Fission Milestone:M5, firefox71 verified, firefox72 verified)

VERIFIED FIXED
Firefox 72
Fission Milestone M5
Tracking Status
firefox71 --- verified
firefox72 --- verified

People

(Reporter: ochameau, Assigned: gl)

References

(Blocks 3 open bugs)

Details

(Whiteboard: dt-fission dt-fission-m1)

Attachments

(1 file)

We should introduce a preference similar to devtools.browsertoolbox.fission, but this time to enable Fission frame debugging directly from regular web toolboxes.

This will depend on bug 1565200 as nothing can work against fission frame until we make connectToFrame to work against Fission frames, via the JS Window Actor API.

A good first usecase to enable this preference would be to also iterate over child inspector front in this code:
https://searchfox.org/mozilla-central/rev/9bb55ae4d808fc48afcf93f99da6a685265b86c6/devtools/shared/fronts/inspector.js#573-577

Blocks: 1578747
Blocks: 1574350
Priority: -- → P2
Whiteboard: dt-fission

When implementing this, we might want to do some modification in inspector front's getNodeFrontFromNodeGrip (check the TODO there)

Assignee: nobody → gl
Status: NEW → ASSIGNED

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)

When implementing this, we might want to do some modification in inspector front's getNodeFrontFromNodeGrip (check the TODO there)

Can you give a bit more details of what you expect to be done here? Thanks!

Flags: needinfo?(nchevobbe)
Priority: P2 → P1

(In reply to Gabriel [:gl] (ΦωΦ) from comment #2)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)

When implementing this, we might want to do some modification in inspector front's getNodeFrontFromNodeGrip (check the TODO there)

Can you give a bit more details of what you expect to be done here? Thanks!

We were saying in reviews that we should also enter this if block fronts/inspector.js#655-661 if the preference isn't true.

Flags: needinfo?(nchevobbe)
Pushed by gluong@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/94a89e567444
Introduce a preference to enable fission frame inspection in DevTools. r=nchevobbe

(In reply to Arthur Iakab [arthur_iakab] from comment #6)

Backed out changeset 94a89e567444 (bug 1578745) for causing multiple devtools failures on browser_dbg-inline-preview.js. CLOSED TREE

Backout revision https://hg.mozilla.org/integration/autoland/rev/d33a6058d98bb3508d237525e15bcb992e9e9315

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=94a89e567444c67a68d4e237356f0a6c85461857

Failure logs: dt7 - https://treeherder.mozilla.org/logviewer.html#?job_id=273326721&repo=autoland
dt4 - https://treeherder.mozilla.org/logviewer.html#?job_id=273326725&repo=autoland

Gabriel can you please take a look?

This turned out to be a keyboard typo in the constants that must've happened while making the review changes.

Flags: needinfo?(gl)
Pushed by gluong@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d7210449434e
Introduce a preference to enable fission frame inspection in DevTools. r=nchevobbe
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72
Blocks: 1593937

Comment on attachment 9104346 [details]
Bug 1578745 - Introduce a preference to enable fission frame inspection in DevTools. r=nchevobbe

Beta/Release Uplift Approval Request

  • User impact if declined: Users can't jump from a logged dom element in the console to the inspector if said element is in an iframe.
    This patch wasn't made to fix anything, but appears to fix Bug 1591757.

An automated test will be added in Bug 1591757 and I'll request the uplift as well there to ensure everything is okay on Beta.

  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See https://bugzilla.mozilla.org/show_bug.cgi?id=1591757#c0
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This patch introduce a new preference, that is false by default.
    The fact that the preference wasn't set was the cause of the issue of Bug 1591757.
  • String changes made/needed:
Attachment #9104346 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Retroactively moving fixed bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to an appropriate Fission Milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Fission Milestone: --- → M5

Comment on attachment 9104346 [details]
Bug 1578745 - Introduce a preference to enable fission frame inspection in DevTools. r=nchevobbe

Fix for bug 1591757 which is a tracked P1, uplift approved for 71 beta 11, thanks.

Attachment #9104346 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1598022

Hello,

I can still reproduce this issue (stated in bug 1591757) on Fx 71.0b11 so I will be marking Fx 71 as affected.

Flags: needinfo?(gl)

(In reply to Daniel Cicas [:dcicas], Release QA from comment #15)

Hello,

I can still reproduce this issue (stated in bug 1591757) on Fx 71.0b11 so I will be marking Fx 71 as affected.

The cases that consterned me seem to be fixed in 71 now, what case might still be affected?

Forwarding this to nchevobbe

Flags: needinfo?(gl) → needinfo?(nchevobbe)

(In reply to Daniel Cicas [:dcicas], Release QA from comment #15)

Hello,

I can still reproduce this issue (stated in bug 1591757) on Fx 71.0b11 so I will be marking Fx 71 as affected.

Daniel, could you tell me what STR you used that fails in 71 and works in 72?

Flags: needinfo?(nchevobbe) → needinfo?(daniel.cicas)

Hello,

Here are the steps I used:

  1. Create a simple HTTP server with both the iframe.html and the testispector.html files on it.
  2. I opened the testinspector.html file in localhost.
  3. Open devtools console and click on the console.log (for lack of a better term the square next to it)

Actual result:
The inspector returns me to the body, not to the iframe nested inside several divs.

Flags: needinfo?(daniel.cicas)

Daniel, I just tested this in 71.0b11 (64-bit) and it works fine to me, at step 3. the h1 node of the iframe is selected in the inspector.
Could you try again please?
Maybe there's a race condition or something?

Flags: needinfo?(daniel.cicas)

Hello,

I tried again and it's working now I must have made a slip up. I am confirming this bug as verified fixed for Fx 71.0b12 and Fx 72.0a1.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(daniel.cicas)
Whiteboard: dt-fission → dt-fission dt-fission-m1
You need to log in before you can comment on or make changes to this bug.