Closed Bug 1639934 Opened 1 year ago Closed 1 year ago

[Fission] Enable devtools.contenttoolbox.fission when fission is enabled

Categories

(DevTools :: General, task)

task

Tracking

(Fission Milestone:M6a, firefox79 verified)

VERIFIED FIXED
Firefox 79
Fission Milestone M6a
Tracking Status
firefox79 --- verified

People

(Reporter: emilio, Assigned: jdescottes)

References

Details

Attachments

(1 file)

Open https://jsfiddle.net/L27uqgbw/, click on "inspect element" in the bottom right frame.

I expected to be able to inspect the iframe, but it inspects the top level document instead.

Tracking DevTools bugs for Fission Nightly milestone (M6c)

Fission Milestone: --- → M6c

Support for Fission features in the regular DevTools Toolbox is behind a pref.
Can you try to enable devtools.contenttoolbox.fission and try again?
For me it works with the preference enabled.

Flags: needinfo?(emilio)

It does! Should those be enabled iff fission is enabled? Not knowing about that pref means that debugging codepens and such is much harder.

Flags: needinfo?(emilio)

Ah, the rules panel doesn't seem to work though.

Initially the rationale was:

  • DevTools might be severely broken with devtools.contenttoolbox.fission = true
  • regular DevTools can still be useful with Fission on, just missing remote frame info

So we decided that it was better to have a separate pref rather than forcing all Fission users to have those experimental/broken DevTools.
But as you said it makes it hard to discover, and the configuration matrix (fission on/off, devtools fission on/off) is somewhat painful.

The end goal is to enable this preference by default (regardless of Fission.autostart), but maybe your suggestion can be a good compromise.
Let's see what the rest of the team thinks.

Alex: what do you think about binding devtools.contenttoolbox.fission to fission.autostart? Is it something we could do right now, or after a certain milestone?

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

Ah, the rules panel doesn't seem to work though.

Some things might be broken, but the rules panel works for me.

Alex: See comment 5

Flags: needinfo?(poirot.alex)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

It does! Should those be enabled iff fission is enabled? Not knowing about that pref means that debugging codepens and such is much harder.

(In reply to Julian Descottes [:jdescottes] from comment #5)

Alex: what do you think about binding devtools.contenttoolbox.fission to fission.autostart? Is it something we could do right now, or after a certain milestone?

The issue with enabling the "work in progress" support of Fission in DevTools is that we don't really know how many things are broken.
We would have to do a bit of testing to evaluate if that's benefitial. That wouldn't be a good trade if we exchange incompleteness with breakages.
May be we should start with a warning display if we see that fission.autostart is enabled?
We could either mention the devtools preference, or bind the pref and still mention that things are unstable.

Honza, Harald, do you have any opinion? I'm bit overloaded. Would you have some time to evaluate the current state of tools with the two fission pref enabled?

Flags: needinfo?(poirot.alex)
Flags: needinfo?(odvarko)
Flags: needinfo?(hkirschner)

Would you have some time to evaluate the current state of tools with the two fission pref enabled?

I have it enabled for 2+ weeks now in my main Nightly profile and have not had obvious breakage from it or experienced that any critical functionality might be not working.

Fission is in dogfooding phase, so everybody using is aware of the "risks" and ready to file bugs. Since we don't know of any blockers right now I'd like to see more devs switching it on and test; or asking QA to help. It does seem that we only need a little more confidence to enable it for Fission dogfooders.

Flags: needinfo?(hkirschner)

Renaming and moving the bug.

I see two options here. Either close this as duplicate of enabling devtools fission. Or try to align the devtools.contenttoolbox.fission pref on the fission.autostart preference.

There are some doubts on the stability of DevTools when devtools.contenttoolbox.fission is on, but maybe it's acceptable to turn it on for users already dogfooding fission.

A proposal could be:

  • make devtools.contenttoolbox.fission default to true on Nightly
  • enable Fission for DevTools in the content toolbox iff both devtools.contenttoolbox.fission = true & fission is enabled

This already means wrapping all access to devtools.contenttoolbox.fission behind a shared helper.
I will try to propose a patch for this.

Type: defect → task
Component: Inspector → General
Summary: [Fission] Inspecting an element in a cross-origin iframe doesn't work. → [Fission] Enable devtools.contenttoolbox.fission when fission is enabled
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED

There are some doubts on the stability of DevTools when devtools.contenttoolbox.fission is on, but maybe it's acceptable to turn it on for users already dogfooding fission.

Agreed. Even if it turns out to be wrong, we'll learn about severe breakage reports and can adjust accordingly.

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/081d5f77c22a
Enable devtools.contenttoolbox.fission by default but gate it behind fission.autostart r=ochameau
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 79
Regressions: 1646636
Regressions: 1646633
Regressions: 1646880
Flags: qe-verify+
Fission Milestone: M6c → M6a

Verified with 79.0b4 on Windows 10, macOS 10.15.5, Ubuntu 18.
With it on true/fission.autostart locked on false - no crashes encountered.
No crashes encountered for 80.0a1 (2020-07-02) as well with both prefs enabled.

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