Enable the CSS compatibility tooltip in Nightly
Categories
(DevTools :: Inspector: Rules, task, P3)
Tracking
(firefox115 fixed)
Tracking | Status | |
---|---|---|
firefox115 | --- | fixed |
People
(Reporter: mtigley, Assigned: nchevobbe)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: dev-doc-needed)
Attachments
(2 files)
Now that the CSS compatibility tooltip is available behind the devtools.inspector.ruleview.inline-compatibility-warning.enabled
pref, we should think about having the feature enabled in Firefox Nightly by default at some point.
Perhaps we can enable the feature in Nightly 82? What do you think, Daisuke and Prateek?
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Updated•4 years ago
|
Comment 1•4 years ago
|
||
Thank you very much for filing this!
I don't see any problem to enable this feature.
This patch enables the experimental CSS compatibility tooltip in
inspector's rules panel to test for stability.
Updated•4 years ago
|
Updated•4 years ago
|
Comment 4•4 years ago
|
||
Backed out for multiple devtools failures due to compatibility issues.
backout: https://hg.mozilla.org/integration/autoland/rev/7c5bd84d0851a0cf5aaededce8e9a701a28cd009
failure logs:
- TEST-UNEXPECTED-FAIL | devtools/client/inspector/markup/test/browser_markup_shadowdom_nested_pick_inspect.js | A promise chain failed to handle a rejection: Connection closed, pending request to server0.conn124.child3/inspectorActor4, type getCompatibility failed https://treeherder.mozilla.org/logviewer?job_id=321478736&repo=autoland&lineNumber=14907
- TEST-UNEXPECTED-FAIL | devtools/client/responsive/test/browser/browser_toolbox_computed_view.js | A promise chain failed to handle a rejection: Connection closed, pending request to server0.conn5.child3/compatibility39, type getCSSDeclarationBlockIssues failed https://treeherder.mozilla.org/logviewer?job_id=321478664&repo=autoland&lineNumber=12938
- TEST-UNEXPECTED-FAIL | devtools/client/inspector/rules/test/browser_rules_add-property-and-reselect.js | A promise chain failed to handle a rejection: Connection closed, pending request to server0.conn0.child3/compatibility47, type getCSSDeclarationBlockIssues failed https://treeherder.mozilla.org/logviewer?job_id=321482153&repo=autoland&lineNumber=4850
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Comment 5•4 years ago
|
||
Julien provided me a few tips on what we can do to solve these test failures. The main issue here is that selecting an element will trigger calls to getCompatibility
/ getCSSDeclarationBlockIssues
and we're not waiting for them to complete before shutting down.
Part of the problem is that the rule view calls updatePropertyCompatibilityIndicator
(an async function) in updatePropertyState
(which is synchronous): https://searchfox.org/mozilla-central/source/devtools/client/inspector/rules/views/text-property-editor.js#786-788. One approach that could help this is to initialize the CompatibilityFront upfront, when the InspectorFront is initialized: https://searchfox.org/mozilla-central/source/devtools/client/fronts/inspector.js#47-50.
The next issue is solving the failures involving getCSSDeclarationBlockIssues
, which needs to call the actor everytime. Julien has suggested the following approaches:
-
Modify the inspector opening sequence to wait for an event emitted at the of
updatePropertyCompatibilityIndicator
-
Use
safeAsyncMethod
to silence these exceptions
The second approach seems to have a few errors still. This might be due to an issue with async fronts (i.e: fronts with a async initialize methods) when they are retrieved via protocol.js instead of getFront
. This issue has been documented in Bug 1677506.
Comment 6•4 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:lelouch.cpp, could you have a look please?
For more information, please visit auto_nag documentation.
Reporter | ||
Comment 7•4 years ago
|
||
I unfortunately didn't have time to experiment with what I mentioned in comment 5 before going on PTO. I'll move the needinfo to myself to put this on my radar this week.
Comment 8•4 years ago
|
||
Now that https://bugzilla.mozilla.org/show_bug.cgi?id=1677660 landed, the compatibility front should initialize synchronously, and I think it can be worth trying to land this again.
Comment 9•4 years ago
|
||
I have pushed the patch to try: https://treeherder.mozilla.org/jobs?repo=try&revision=a0b22199f8c6250a8e0ac0fcef8f6e34389d58c0
It will need to be rebased on phabricator before landing though. If try is green we'll see if Kriyszig is still around to update the diff on Phabricator.
Comment 10•4 years ago
|
||
Thanks for the try push Julian. I'm a little occupied this week so I'll rebase the patch over the weekend and update on Phabricator.
Thank you for digging into this.
Comment 11•4 years ago
|
||
(In reply to Kriyszig from comment #10)
Thanks for the try push Julian. I'm a little occupied this week so I'll rebase the patch over the weekend and update on Phabricator.
Thank you for digging into this.
Sure thing!
Looks like we still have a lot of failures, but they are now all related to the second issue, ie calls to getCSSDeclarationBlockIssues
coming from a sync method. Looking at the try failures, the calls to getCSSDeclarationBlockIssues occur from a variety of stacks (element selection, mutation etc...).
For now I don't have a good idea on how to handle this other than try/catching errors.
Here's a TRY push with a patch to swallow errors: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=RIy70tEjT4SK8tBIADESMQ.0&revision=1627610490af1ace7832b53a94eedeefd7e1de13
Some known intermittents but a concerning issue on Linux debug on devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_contextmenu.js . Will have to be investigated before landing.
Comment 12•3 years ago
|
||
The bug assignee didn't login in Bugzilla in the last 7 months.
:jdescottes, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•3 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 13•2 years ago
|
||
I pushed again to see if we have similar issues today
With Julian's patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1659498#c11 , we get a few failures still: https://treeherder.mozilla.org/jobs?repo=try&revision=15cf50cc7a843fb5fc2923a30785fcf782ef07d4
There's not a ton of tests failing an from what I can tell, they all have the same error:
A promise chain failed to handle a rejection: Connection closed, pending request to server0.conn0.windowGlobal6442450976/compatibility40, type getCSSDeclarationBlockIssues failed
There's probably something we can tweak to avoid this, I'll investigate
Assignee | ||
Comment 14•2 years ago
|
||
This was causing test failures on TRY as we had rejections in getCompatibilityIssues
when the page navigates or the toolbox is being destroyed.
To workaround this, we wrap the call in a try/catch block where we swallow the
error if the toolbox is being destroyed or the page navigates.
Comment 15•1 years ago
|
||
Comment 16•1 years ago
|
||
Backed out for causing compatibility related failures.
Comment 17•1 years ago
|
||
Comment 19•1 years ago
|
||
bugherder |
Description
•