Added DownloadItem .OriginalUrl #1877
Conversation
…Url" in CEF. Added a sidebar to the example app which shows the DownloadItems as received by CefSharp.Core/Internals/TypeConversion.h Added a View menu for hiding/showing the regular sidebar and the new download info sidebar.
| @@ -24,6 +24,7 @@ public static class CefExample | |||
| public const string BasicSchemeTestUrl = "custom://cefsharp/SchemeTest.html"; | |||
| public const string ResponseFilterTestUrl = "custom://cefsharp/ResponseFilterTest.html"; | |||
| public const string DraggableRegionTestUrl = "custom://cefsharp/DraggableRegionTest.html"; | |||
| public const string DownloadHandlerTestUrl = "custom://cefsharp/DownloadHandlerTest.html"; | |||
There was a problem hiding this comment.
Sorry, meant to revert this. I had added a new test html file but it didn't turn out to be a very good addition.
| @@ -14,7 +14,7 @@ | |||
| <BooleanToVisibilityConverter x:Key="BooleanToVisibilityConverter" /> | |||
| <ObjectDataProvider x:Key="BitmapScalingModeEnum" MethodName="GetValues" ObjectType="{x:Type system:Enum}"> | |||
| <ObjectDataProvider.MethodParameters> | |||
| <x:Type TypeName="BitmapScalingMode"/> | |||
| <x:Type TypeName="BitmapScalingMode" /> | |||
There was a problem hiding this comment.
Please revert whitespace changes
| @@ -44,8 +44,7 @@ | |||
| GotKeyboardFocus="OnTextBoxGotKeyboardFocus" | |||
| GotMouseCapture="OnTextBoxGotMouseCapture"> | |||
| <TextBox.InputBindings> | |||
| <KeyBinding Key="Enter" | |||
| Command="{Binding GoCommand}" /> | |||
| <KeyBinding Key="Enter" Command="{Binding GoCommand}" /> | |||
There was a problem hiding this comment.
Please revert whitespace changes
|
|
||
| private void UpdateDownloadAction(string downloadAction, DownloadItem downloadItem) | ||
| { | ||
| this.Dispatcher.Invoke(() => |
There was a problem hiding this comment.
Is there a reason to update in a sync fashion? UI updates should be async where possible.
There was a problem hiding this comment.
Good catch. Changed to InvokeAsync now.
| @@ -16,6 +16,8 @@ | |||
| *.swp | |||
| *.user | |||
| *.xml | |||
| *.VC.opendb | |||
There was a problem hiding this comment.
Would ideally be in it's own PR, though leave it for now. (Doesn't relate to the feature your adding).
| public void OnBeforeDownload(IBrowser browser, DownloadItem downloadItem, IBeforeDownloadCallback callback) | ||
| { | ||
| if (this.OnBeforeDownloadFired != null) |
There was a problem hiding this comment.
Would prefer firing of event handlers to look like below for consistency and best practice.
var handler = LoadingStateChanged;
if (handler != null)
{
handler(this, args);
}I know this is a demo, though I prefer to take a reference to a handler before executing as it's possible in a multithreaded environment for the handler reference to be cleared between the null check and executing. There's no
There was a problem hiding this comment.
I update to match that style. Presumably using a null-conditional would also be okay? E.g.
OnBeforeDownloadFired?.Invoke(this, downloadItem);
| @@ -19,7 +30,10 @@ public void OnBeforeDownload(IBrowser browser, DownloadItem downloadItem, IBefor | |||
|
|
|||
| public void OnDownloadUpdated(IBrowser browser, DownloadItem downloadItem, IDownloadItemCallback callback) | |||
| { | |||
|
|
|||
| if (this.OnDownloadUpdatedFired != null) | |||
There was a problem hiding this comment.
For consistency this has typically only been used when referencing a private variable.
amaitland
changed the title
|
Thanks for the feedback Alex! I have pushed fixes for those problems. |
|
Thanks, merged 👍 |
|
Awesome, thanks Alex! |
This property is very valuable and I need it in the project I am working on right now!
Also:
Added a sidebar to the example app which shows the DownloadItems as received by CefSharp.Core/Internals/TypeConversion.h
Added a View menu for hiding/showing the regular sidebar and the new download info sidebar.