[FileExplorerPreview] Move everything from WebBrowser to WebView2 #17588

stefansjfw · 2022-04-07T14:19:32Z

Summary of the Pull Request

FIXED: Why WIP? Same issue as with Monaco, can't test it properly because of all of the async stuff going on while initializing WebView environment. Looking into this...

What is this about:
OPEN QUESTIONS:

Do we want context menu? EDIT: Context menu disabled

WebView allows opening link in the Preview Pane. I guess we don't want that, I just need to find a way to disable it.
@Aaron-Junker Do you know how to do this? EDIT: Links are now opened in default browser

What is included in the PR:

How does someone test / validate:

Build installer
Confirm that all p preview and thumbnail handlers works correctly

Quality Checklist

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

Aaron-Junker · 2022-04-07T14:31:26Z

OPEN QUESTIONS:

Do we want context menu?

I think if we want one we should create our own because of elemts like back and forward and inspect which we don't want

WebView allows opening link in the Preview Pane. I guess we don't want that, I just need to find a way to disable it.
@Aaron-Junker Do you know how to do this?

My suggestion would be to execute JavaScript from the C# code (https://docs.microsoft.com/en-us/microsoft-edge/webview2/how-to/javascript) which adds a <base target="_blank" /> tag. This should open the links in a new window.

Open links in default browser

stefansjfw · 2022-04-08T07:53:01Z

OPEN QUESTIONS:

Do we want context menu?

I think if we want one we should create our own because of elemts like back and forward and inspect which we don't want

WebView allows opening link in the Preview Pane. I guess we don't want that, I just need to find a way to disable it.
@Aaron-Junker Do you know how to do this?

My suggestion would be to execute JavaScript from the C# code (https://docs.microsoft.com/en-us/microsoft-edge/webview2/how-to/javascript) which adds a <base target="_blank" /> tag. This should open the links in a new window.

I disabled context menu for now, as it didn't exist previously.
Also links are now opened in default browser.

Remove WebBrowser related stuff

Disable javascript dialogs in WebView2 for Svg thumbnail and preview

stefansjfw · 2022-04-13T11:46:23Z

This should be fully ready for review now. Tests are fixed.
@Aaron-Junker Are willing to check what I did with tests and add tests for Monaco in the same fashion? (e.g. in MarkdownPreviewHandlerUnitTests)

Aaron-Junker · 2022-04-13T11:50:08Z

This should be fully ready for review now. Tests are fixed.
@Aaron-Junker Are willing to check what I did with tests and add tests for Monaco in the same fashion? (e.g. in MarkdownPreviewHandlerUnitTests)

Maybe I can do this this week(end). Do you want it in this PR?

stefansjfw · 2022-04-13T11:51:20Z

Maybe I can do this this week(end). Do you want it in this PR?

Great, thanks! If this doesn't get merged until then, then you can add it to this PR.

jaimecbernardo · 2022-04-13T14:00:38Z

This issue can be linked: #15764

jaimecbernardo

Some nits.
I also see some of the tests have been removed. No longer needed or is there no equivalent call in WebView 2?

Do we need to add something more to the installer for this to work?

jaimecbernardo · 2022-04-13T14:06:13Z

src/modules/previewpane/SvgThumbnailProvider/SvgThumbnailProvider.cs

+                {
+                    // We are not the appropriate size for caller.  Resize now while
+                    // respecting the aspect ratio.
+                    float scale = Math.Min((float)cx / thumbnail.Width, (float)cx / thumbnail.Height);


Should we add a divide by 0 check here?

jaimecbernardo · 2022-04-13T14:06:51Z

src/modules/previewpane/UnitTests-MarkdownPreviewHandler/MarkdownPreviewHandlerTest.cs

+
+                while (markdownPreviewHandlerControl.Controls.Count == 0 && Environment.TickCount < beforeTick + TenSecondsInMilliseconds)
+                {
+                    Application.DoEvents();


Perhaps a little sleep here to not use 100% CPU would be good.

added sleeps everywhere

src/modules/previewpane/UnitTests-MarkdownPreviewHandler/MarkdownPreviewHandlerTest.cs

jaimecbernardo · 2022-04-14T12:10:33Z

Actually, #17527 is not fixed by this, my bad.

stefansjfw · 2022-04-14T13:29:09Z

Actually, #17527 is not fixed by this, my bad.

removed

Aaron-Junker · 2022-04-14T14:33:36Z

@stefansjfw When I do it. Is it ok to just commit against your branch?

jaimecbernardo

LGTM! Thank you!

Aaron-Junker · 2022-04-14T15:27:48Z

Ok I guess then I make my own PR.

jaimecbernardo · 2022-04-14T15:27:51Z

@stefansjfw When I do it. Is it ok to just commit against your branch?

@Aaron-Junker , let's make that a separate PR ;)
Thank you

zakius · 2022-05-03T18:29:20Z

using Blink is a terrible idea due to text rendering and image scaling issues

take a look at this comparison, upper one is rendered by Blink and is barely readable while the lower one is rendered using the default preview and looks much better

Move MarkdownPreviewHandler from WebBrowser to WebView2

fb2e74b

stefansjfw requested review from crutkas, yuyoyuppe, SeraphimaZykova, jaimecbernardo and Aaron-Junker April 7, 2022 14:19

stefansjfw changed the title ~~[Draft] [FileExplorerPreview] Move MarkdownPreviewHandler from WebBrowser to WebView2~~ [WIP] [FileExplorerPreview] Move MarkdownPreviewHandler from WebBrowser to WebView2 Apr 7, 2022

Disable context menu

db2e7bb

Open links in default browser

This comment was marked as resolved.

Sign in to view

stefansjfw added 6 commits April 8, 2022 09:55

Update expect.txt

eeb25d6

Move SvgPreviewHandler from WebBrowser to WebView2

0233869

Migrate SvgThumbnailProvider from WebBrowser to WebView2

ec61dcf

Migrate CustomControlTest to WebView2

bfd2e9d

Remove WebBrowser related stuff

Update tests

a6d76b5

Revert GetThumbnail return value

5b7baff

Disable javascript dialogs in WebView2 for Svg thumbnail and preview

stefansjfw changed the title ~~[WIP] [FileExplorerPreview] Move MarkdownPreviewHandler from WebBrowser to WebView2~~ [FileExplorerPreview] Move everything from WebBrowser to WebView2 Apr 13, 2022

This comment was marked as resolved.

Sign in to view

expect.txt

3835fe7

stefansjfw force-pushed the stefan/markdown_preview_webview2 branch from 8333726 to 8e9f95c Compare April 13, 2022 13:52

Increase timeout for Markdown tests

2496696

stefansjfw force-pushed the stefan/markdown_preview_webview2 branch from 8e9f95c to 2496696 Compare April 13, 2022 13:55

jaimecbernardo reviewed Apr 13, 2022

View reviewed changes

This was referenced Apr 13, 2022

Certain Inkscape SVG files do not preview #17527

Closed

Some SVG icons are not shown in preview #17181

Closed

stefansjfw added 2 commits April 14, 2022 15:38

Add sleeps

3f6b259

Add zero check

932a553

jaimecbernardo approved these changes Apr 14, 2022

View reviewed changes

jaimecbernardo merged commit 88517bf into main Apr 14, 2022

crutkas deleted the stefan/markdown_preview_webview2 branch April 20, 2022 18:45

davidegiacometti mentioned this pull request Apr 21, 2022

Markdown Preview : Clickable links #1834

Closed

razeedazee mentioned this pull request Aug 31, 2023

Markdown Preview: Show images #1837

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FileExplorerPreview] Move everything from WebBrowser to WebView2 #17588

[FileExplorerPreview] Move everything from WebBrowser to WebView2 #17588

stefansjfw commented Apr 7, 2022 •

edited

Aaron-Junker commented Apr 7, 2022

stefansjfw commented Apr 8, 2022

This comment was marked as resolved.

This comment was marked as resolved.

stefansjfw commented Apr 13, 2022

Aaron-Junker commented Apr 13, 2022

stefansjfw commented Apr 13, 2022

jaimecbernardo commented Apr 13, 2022

jaimecbernardo left a comment

jaimecbernardo Apr 13, 2022

stefansjfw Apr 14, 2022

jaimecbernardo Apr 13, 2022

stefansjfw Apr 14, 2022

jaimecbernardo commented Apr 14, 2022

stefansjfw commented Apr 14, 2022

Aaron-Junker commented Apr 14, 2022

jaimecbernardo left a comment

Aaron-Junker commented Apr 14, 2022

jaimecbernardo commented Apr 14, 2022

zakius commented May 3, 2022

[FileExplorerPreview] Move everything from WebBrowser to WebView2 #17588

[FileExplorerPreview] Move everything from WebBrowser to WebView2 #17588

Conversation

stefansjfw commented Apr 7, 2022 • edited

Summary of the Pull Request

Quality Checklist

Contributor License Agreement (CLA)

Aaron-Junker commented Apr 7, 2022

stefansjfw commented Apr 8, 2022

This comment was marked as resolved.

This comment was marked as resolved.

stefansjfw commented Apr 13, 2022

Aaron-Junker commented Apr 13, 2022

stefansjfw commented Apr 13, 2022

jaimecbernardo commented Apr 13, 2022

jaimecbernardo left a comment

Choose a reason for hiding this comment

jaimecbernardo Apr 13, 2022

Choose a reason for hiding this comment

stefansjfw Apr 14, 2022

Choose a reason for hiding this comment

jaimecbernardo Apr 13, 2022

Choose a reason for hiding this comment

stefansjfw Apr 14, 2022

Choose a reason for hiding this comment

jaimecbernardo commented Apr 14, 2022

stefansjfw commented Apr 14, 2022

Aaron-Junker commented Apr 14, 2022

jaimecbernardo left a comment

Choose a reason for hiding this comment

Aaron-Junker commented Apr 14, 2022

jaimecbernardo commented Apr 14, 2022

zakius commented May 3, 2022

stefansjfw commented Apr 7, 2022 •

edited