-
Notifications
You must be signed in to change notification settings - Fork 636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DYN-5208-DynamoRevit-Crash-Fix #13245
Changes from all commits
f269f10
f6f8697
45e845b
e101e8a
b37dcc1
2729c08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -430,6 +430,17 @@ private Step CreateStep(Step jsonStepInfo, HostControlInfo hostControlInfo, int | |
var formattedText = Res.ResourceManager.GetString(jsonStepInfo.StepContent.FormattedText); | ||
var title = Res.ResourceManager.GetString(jsonStepInfo.StepContent.Title); | ||
|
||
DirectoryInfo userDataFolder = null; | ||
if (dynamoViewModel != null) | ||
{ | ||
var pathManager = dynamoViewModel.Model.PathManager; | ||
|
||
//When executing Dynamo as Sandbox or inside any host like Revit, FormIt, Civil3D the WebView2 cache folder will be located in the AppData folder | ||
var docsDir = new DirectoryInfo(pathManager.UserDataDirectory); | ||
userDataFolder = docsDir.Exists ? docsDir : null; | ||
|
||
} | ||
|
||
switch (jsonStepInfo.StepType) | ||
{ | ||
case Step.StepTypes.TOOLTIP: | ||
|
@@ -446,6 +457,11 @@ private Step CreateStep(Step jsonStepInfo, HostControlInfo hostControlInfo, int | |
Title = title | ||
} | ||
}; | ||
var popupWindow = newStep.stepUIPopup as PopupWindow; | ||
if(popupWindow != null && hostControlInfo.HtmlPage != null && !string.IsNullOrEmpty(hostControlInfo.HtmlPage.FileName)) | ||
{ | ||
popupWindow.WebBrowserUserDataFolder = userDataFolder != null ? userDataFolder.FullName : string.Empty; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this code here? Shouldn't this be set one time only at the Welcome step? Is it because all of our Welcome steps not leveraging WebView2? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @QilongTang
2nd question.
3rd question.
commit: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @RobertGlobant20 are there any MSDN docs on how these webview2 startup options affect other instances of WebView2 in the same process? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mjkkirschner MS documentation doesn't talk specifically about that but I think the startup options are specific for each WebView2 instance. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @RobertGlobant20 thanks- I'm also curious because inside of Hosts like Revit, they will use WebView2 and so will the other addins running in the Host process. It's the wild west! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mjkkirschner well, we asked Revit about it but for my understanding they are still in the process of adopting WebView2 then they haven't deal with this feature yet so if Revit team decide to use another cache folder in AppData for WebView2 we can evaluate a change in Dynamo to use the same than them, another thing is that we need to ask if this feature of enable/disable cache for WebView2 will be enabled in the future (since I didn't find anything about it). |
||
break; | ||
case Step.StepTypes.SURVEY: | ||
newStep = new Survey(hostControlInfo, jsonStepInfo.Width, jsonStepInfo.Height) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to double check on this if there is a better way. I will get back to you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for Sandbox, this pathManager.HostApplicationDirectory would be null? I think this only means this is not an in process integration, for case like FormIt, where it is out of process but still delivered in programe files, this may still not work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@QilongTang
For other products like FormIt or Civil not sure which is the path of the Dynamo "fallback_doc" folder, I followed the implementation that I found in PackageDocumentationManager.cs (check link below) then if the fallback_doc folder location is different for each host (Revit, Civil, Formit) then the implementation done in PackageDocumentationManager is not working or hostDynamoFallbackDocPath = null or there is another process creating the folder and moving all the md/jpg files to the HostApplicationDirectory path.
Should I install FormIt and Civil so I can test if images are shown in DocumentationBrowser?
Dynamo/src/DocumentationBrowserViewExtension/PackageDocumentationManager.cs
Line 66 in 79ebac3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@QilongTang now the WebView2 cache folder will be always created in AppData, see the next commit:
e101e8a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RobertGlobant20 Is the comment above still accurate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@QilongTang fixed comment about Revit in the next commit:
2729c08