Open core-ai-bot opened 3 years ago
core-ai-bot
commented
3 years ago 
Crap, this appears to be a browser repaint bug. The CSS seems fine as-is, and kicking it to update by adding and removing a color: red on any given node (i.e. a no-op style change) causes that one node to start displaying correctly.
This immediately makes this CEF update seem much more risky :-(
core-ai-bot
commented
3 years ago
On a hunch, I tried removing the "quiet scrollbars" stuff, but that didn't fix it.
However, if I pull #project-files-container out of #sidebar and dump it as a following sibling, the issue appears to go away. Seems worth ripping out pieces of styles & layout stuff from #sidebar to try to identify what's acting as a trigger.
core-ai-bot
commented
3 years ago
Ugh, not looking good as far as an easy smoking gun. I reduced it a bit to this:
<div class="main-view">
<div id="sidebar" class="sidebar panel">
<div class="working-set-splitview-btn btn-alt-quiet"></div>
<div class="working-set-option-btn btn-alt-quiet"></div>
<div id="working-set-list-container">
</div>
<div id="project-files-header">
<span id="project-title" class="title"></span>
</div>
</div>
<div class="fake-sidebar">
<div id="project-files-container">
<!-- This will contain a dynamically generated <ul> hierarchy at runtime -->
</div>
</div>and
.fake-sidebar {
width: 200px;
}Removing the width there will make the bug go away. Uhhh...?
Notably, this removes the flexbox layout that is on a normal .sidebar. Also, it doesn't matter what the width value is -- I can make it 1000px and the labels are still all missing.
core-ai-bot
commented
3 years ago
Interestingly, Chrome stable just hit 39.0.2171.65 and this new CEF is also 39.0.2171.65 -- so one thing we can do is run Brackets in the browser and see if the same bug repros there. I'll start dusting off my in-browser branch so we have it available for that...
core-ai-bot
commented
3 years ago 
Scary. If this does repro in Chrome stable, then we'd be pretty sure to get a fix.
core-ai-bot
commented
3 years ago 
There's no screen grab, unfortunately, but #8133 is what we were seeing with 1916
core-ai-bot
commented
3 years ago
Sadly, I cannot get this to repro in browser yet. If anyone else would like to give it a try, I've pushed up pflynn/in-browser-dummy-subfolders -- a sub-branch of the in-browser version that has a more complex file tree, and a slight async delay on IO operations to approximate the timing seen in real Brackets.
core-ai-bot
commented
3 years ago
Aha! I can repro this with the simple in-browser version in the CEF 2171 brackets-shell. In theory that code couldn't rely on any Brackets-only shell APIs, so it should work as a repo case in vanilla cefclient too (but still not Chrome itself) -- perfect testcase for Marshall.
@JeffryBooher How hard is it to make a vanilla cefclient build? Is that in our grunt script or anything? If not, would you mind posting a copy somewhere so I could play with it?
core-ai-bot
commented
3 years ago
@peterflynn you will find it in your deps/cef folder under brackets-shell. Open cefclient.vcxproj in Visual Studio and build/run it. That will get you a basic web browser with an address bar that you should be able to enter a file:// URL to your test page
core-ai-bot
commented
3 years ago
@JeffryBooher I get a linker error: "cannot open input file '...\deps\cef\out\Debug\lib\libcef_dll_wrapper.lib'" (not sure why it's looking there since I had selected the Release configuration, but \lib is empty in both Debug & Release anyway...)
core-ai-bot
commented
3 years ago
I found a libcef_dll_wrapper.sln in the same folder, but if I open that and make a Debug build (still no idea why it's not looking in the Release folder), then I get more link errors in the cefclient project. Most of them are "mismatch detected for '_ITERATOR_DEBUG_LEVEL': value '2' doesn't match value '0' in cefclient.obj." Any ideas?
core-ai-bot
commented
3 years ago
Aha, looks like you just can't make Release builds of cefclient. I got a Debug build working though -- and hurrah!, the labels are consistently broken there too.
Hopefully this is good enough to package into a testcase now, since there's a clear rendering diff between CEF and Chromium.
core-ai-bot
commented
3 years ago
@peterflynn my bad, you were supposed to open cefclient2010.sln in Visual Studio which will build all of the dependencies.
core-ai-bot
commented
3 years ago 
I found the culprit. If I comment out the following CSS properties that can trigger the GPU mode, then the issue is fixed.
/** Overriding jsTreeTheme.less
*/
.jstree-brackets .jstree-no-dots .jstree-open > ins {
background-position: 7px -8px;
/* -webkit-transform: translateZ(0) rotate(90deg);
-webkit-transition: -webkit-transform 190ms cubic-bezier(.01, .91, 0, .99);
-webkit-filter: drop-shadow(1px 0 1px`@`bc-shadow);*/
/* .dark & {
-webkit-filter: drop-shadow(1px 0 1px`@`dark-bc-shadow);
}*/
}
.jstree-brackets .jstree-no-dots .jstree-closed > ins {
background-position: 7px -8px;
/* -webkit-transform: translateZ(0); Need this to make sure that the svg isn't blurry on retina.
-webkit-transition: -webkit-transform 90ms cubic-bezier(.01, .91, 0, .99);
-webkit-filter: drop-shadow(1px 0 1px`@`bc-shadow);*/
/* .dark & {
-webkit-filter: drop-shadow(1px 0 1px`@`dark-bc-shadow);
}*/
}So this is definitely related to #9978 which is caused by translateZ, but we also need to report that -webkit-filter property can trigger the GPU mode and causes text not to render.
core-ai-bot
commented
3 years ago
Actually, the real culprit is not -webkit-filter as mentioned in my previous comment. I still see the very first item (either file/folder) of the tree not rendered with the above commented code. If I instead comment out the following lines in jsTreeTheme.less file, then the entire tree is rendered.
.jstree-brackets li > a {
/* `@`jstree-icon-text-overlap: 2px;
padding-left: (@jstree-sprite-size -`@`jstree-icon-backindent -`@`jstree-icon-text-overlap + 10000px);
margin-left: -10000px;*/
display: block;
}It seems like our trick to render the tree offscreen (in negative coordinate) and then bring it back with margin-left and padding-left properties are ignored by latest Chromium.
core-ai-bot
commented
3 years ago
In my in-browser/cefclient testcase, removing -webkit-filter fixes the persistent bug, but the labels still briefly disappear whenever the tree is updated. Removing the huge padding-left and margin-left from the rule above fully fixes the bug.
I searched for other places where we use large values in CSS, and luckily nothing else looks too similar to this. (.dummy-text uses a very large negative top, but it's meant to stay offscreen -- not be pushed back onscreen by a similarly large positive value).
core-ai-bot
commented
3 years ago
I filed https://code.google.com/p/chromiumembedded/issues/detail?id=1452 on the CEF bug, but it does appear that we can safely work around it in this case by removing the margin/padding hack, which is no longer needed with our new React-based tree widget.
core-ai-bot
commented
3 years ago
Confirmed fixed in Brackets. Although the underlying CEF/Chromium issue is still present (see bug linked above), I don't think this needs to stay open tracking a fix since we don't have reason to believe anything else in Brackets will trigger that bug. Closing.
Open the brackets project and collapse all top-level nodes all disclosure triangles.
Expanding the tree node shows it but clicking other node will hide it again. Feels random.
Also, the
context-selectedstyles are wrong.